fix: wait for the stopped state in mode:stop before returning

dist/index.js

@@ -104662,6 +104662,7 @@ const {
   ModifyInstanceAttributeCommand,
   AssociateAddressCommand,
   waitUntilInstanceRunning,
+  waitUntilInstanceStopped,
 } = __nccwpck_require__(5193);
 const fs = __nccwpck_require__(9896);
 const core = __nccwpck_require__(7484);
@@ -105835,11 +105836,32 @@ async function warmStartInstance(instanceId, { userData, label }) {
 }
 
 // Stop (not terminate) an instance so it can be reused from the pool.
+//
+// Blocks until the instance actually reaches the `stopped` state before
+// returning. StopInstances alone only moves it to `stopping`, and a warm-pool
+// lookup that runs during that window (findStoppedPoolInstance filters on
+// instance-state-name=stopped) sees an empty pool and cold-launches a
+// duplicate instance the reaper won't drain until its stopped-max-age passes.
+// Waiting here makes "mode: stop finished" imply "instance is reusable", which
+// is the serialization boundary queued follow-up workflow runs rely on.
 async function stopInstanceById(instanceId) {
   const client = ec2Client();
   await withRetry('stop_instance', () => client.send(new StopInstancesCommand({ InstanceIds: [instanceId] })));
   log.info('stop_instance', { instance_id: instanceId });
-  core.info(`AWS EC2 instance ${instanceId} is stopped (warm pool)`);
+  const start = Date.now();
+  log.info('wait_for_instance_stopped', { instance_id: instanceId });
+  try {
+    await waitUntilInstanceStopped(
+      { client, maxWaitTime: 300 },
+      { InstanceIds: [instanceId] },
+    );
+    log.info('wait_for_instance_stopped', { instance_id: instanceId, elapsed_ms: Date.now() - start });
+    core.info(`AWS EC2 instance ${instanceId} is stopped (warm pool)`);
+  } catch (error) {
+    log.error('wait_for_instance_stopped', { instance_id: instanceId, error: error.name, message: error.message });
+    core.error(`AWS EC2 instance ${instanceId} did not reach the stopped state within the wait window`);
+    throw error;
+  }
 }
 
 // Read the reuse-cycle counter tag for an instance (0 when absent).

src/aws.js

@@ -12,6 +12,7 @@ const {
   ModifyInstanceAttributeCommand,
   AssociateAddressCommand,
   waitUntilInstanceRunning,
+  waitUntilInstanceStopped,
 } = require('@aws-sdk/client-ec2');
 const fs = require('fs');
 const core = require('@actions/core');
@@ -1185,11 +1186,32 @@ async function warmStartInstance(instanceId, { userData, label }) {
 }
 
 // Stop (not terminate) an instance so it can be reused from the pool.
+//
+// Blocks until the instance actually reaches the `stopped` state before
+// returning. StopInstances alone only moves it to `stopping`, and a warm-pool
+// lookup that runs during that window (findStoppedPoolInstance filters on
+// instance-state-name=stopped) sees an empty pool and cold-launches a
+// duplicate instance the reaper won't drain until its stopped-max-age passes.
+// Waiting here makes "mode: stop finished" imply "instance is reusable", which
+// is the serialization boundary queued follow-up workflow runs rely on.
 async function stopInstanceById(instanceId) {
   const client = ec2Client();
   await withRetry('stop_instance', () => client.send(new StopInstancesCommand({ InstanceIds: [instanceId] })));
   log.info('stop_instance', { instance_id: instanceId });
-  core.info(`AWS EC2 instance ${instanceId} is stopped (warm pool)`);
+  const start = Date.now();
+  log.info('wait_for_instance_stopped', { instance_id: instanceId });
+  try {
+    await waitUntilInstanceStopped(
+      { client, maxWaitTime: 300 },
+      { InstanceIds: [instanceId] },
+    );
+    log.info('wait_for_instance_stopped', { instance_id: instanceId, elapsed_ms: Date.now() - start });
+    core.info(`AWS EC2 instance ${instanceId} is stopped (warm pool)`);
+  } catch (error) {
+    log.error('wait_for_instance_stopped', { instance_id: instanceId, error: error.name, message: error.message });
+    core.error(`AWS EC2 instance ${instanceId} did not reach the stopped state within the wait window`);
+    throw error;
+  }
 }
 
 // Read the reuse-cycle counter tag for an instance (0 when absent).

tests/warmpool.test.js

@@ -18,6 +18,7 @@ jest.mock('@aws-sdk/client-ec2', () => ({
   ModifyInstanceAttributeCommand: jest.fn((p) => ({ __command: 'ModifyInstanceAttribute', ...p })),
   AssociateAddressCommand: jest.fn((p) => ({ __command: 'AssociateAddress', ...p })),
   waitUntilInstanceRunning: jest.fn(),
+  waitUntilInstanceStopped: jest.fn(),
 }));
 jest.mock('../src/retry', () => ({ withRetry: (_step, fn) => fn() }));
 jest.mock('../src/config', () => ({
@@ -101,11 +102,27 @@ describe('warmStartInstance', () => {
 });
 
 describe('stopInstanceById', () => {
-  test('sends StopInstances', async () => {
+  const { waitUntilInstanceStopped } = require('@aws-sdk/client-ec2');
+
+  beforeEach(() => waitUntilInstanceStopped.mockReset());
+
+  test('sends StopInstances, then waits for the stopped state', async () => {
     mockSend.mockResolvedValue({});
+    waitUntilInstanceStopped.mockResolvedValueOnce({ state: 'SUCCESS' });
     await aws.stopInstanceById('i-9');
     expect(commandsSent()).toEqual(['StopInstances']);
     expect(mockSend.mock.calls[0][0].InstanceIds).toEqual(['i-9']);
+    // The waiter is the whole point: returning at `stopping` lets a queued
+    // follow-up run see an empty pool and cold-launch a duplicate instance.
+    expect(waitUntilInstanceStopped).toHaveBeenCalledTimes(1);
+    expect(waitUntilInstanceStopped.mock.calls[0][1]).toEqual({ InstanceIds: ['i-9'] });
+  });
+
+  test('throws when the instance never reaches stopped', async () => {
+    mockSend.mockResolvedValue({});
+    waitUntilInstanceStopped.mockRejectedValueOnce(new Error('TimeoutError'));
+    await expect(aws.stopInstanceById('i-9')).rejects.toThrow('TimeoutError');
+    expect(commandsSent()).toEqual(['StopInstances']);
   });
 });