From 07e96d6955ee66ace236bfef77deac1471a2363d Mon Sep 17 00:00:00 2001
From: Shiv <92113562+shivdesh@users.noreply.github.com>
Date: Tue, 10 Mar 2026 14:20:12 -0700
Subject: [PATCH] fix(scale-up): prevent negative TotalTargetCapacity when
 runners exceed maximum

When pool and scale-up lambdas run concurrently, currentRunners can
temporarily exceed maximumRunners. This caused the calculation
`maximumRunners - currentRunners` to produce a negative value, which
was then passed to EC2 CreateFleet API, resulting in:

  InvalidTargetCapacitySpecification: TotalTargetCapacity should not be negative.

This fix wraps the calculation with Math.max(0, ...) to ensure we never
attempt to create a negative number of runners.

Fixes race condition between pool-lambda and scale-up-lambda.
---
 .../src/scale-runners/scale-up.test.ts         | 18 ++++++++++++++++++
 .../src/scale-runners/scale-up.ts              |  4 +++-
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/lambdas/functions/control-plane/src/scale-runners/scale-up.test.ts b/lambdas/functions/control-plane/src/scale-runners/scale-up.test.ts
index 458d89763e..c4798a6c80 100644
--- a/lambdas/functions/control-plane/src/scale-runners/scale-up.test.ts
+++ b/lambdas/functions/control-plane/src/scale-runners/scale-up.test.ts
@@ -228,6 +228,24 @@ describe('scaleUp with GHES', () => {
       expect(mockOctokit.actions.createRegistrationTokenForRepo).not.toBeCalled();
     });
 
+    it('does not create runners when current runners exceed maximum (race condition)', async () => {
+      process.env.RUNNERS_MAXIMUM_COUNT = '5';
+      process.env.ENABLE_EPHEMERAL_RUNNERS = 'false';
+      // Simulate race condition where pool lambda created more runners than max
+      mockListRunners.mockImplementation(async () =>
+        Array.from({ length: 10 }, (_, i) => ({
+          instanceId: `i-${i}`,
+          launchTime: new Date(),
+          type: 'Org',
+          owner: TEST_DATA_SINGLE.repositoryOwner,
+        })),
+      );
+      await scaleUpModule.scaleUp(TEST_DATA);
+      // Should not attempt to create runners (would be negative without fix)
+      expect(createRunner).not.toBeCalled();
+      expect(mockOctokit.actions.createRegistrationTokenForOrg).not.toBeCalled();
+    });
+
     it('does create a runner if maximum is set to -1', async () => {
       process.env.RUNNERS_MAXIMUM_COUNT = '-1';
       process.env.ENABLE_EPHEMERAL_RUNNERS = 'false';
diff --git a/lambdas/functions/control-plane/src/scale-runners/scale-up.ts b/lambdas/functions/control-plane/src/scale-runners/scale-up.ts
index 759be95089..d3c20c8329 100644
--- a/lambdas/functions/control-plane/src/scale-runners/scale-up.ts
+++ b/lambdas/functions/control-plane/src/scale-runners/scale-up.ts
@@ -438,12 +438,14 @@ export async function scaleUp(payloads: ActionRequestMessageSQS[]): Promise<stri
     });
 
     // Calculate how many runners we want to create.
+    // Use Math.max(0, ...) to ensure we never attempt to create a negative number of runners,
+    // which can happen when currentRunners exceeds maximumRunners due to pool/scale-up race conditions.
     const newRunners =
       maximumRunners === -1
         ? // If we don't have an upper limit, scale up by the number of new jobs.
           scaleUp
         : // Otherwise, we do have a limit, so work out if `scaleUp` would exceed it.
-          Math.min(scaleUp, maximumRunners - currentRunners);
+          Math.max(0, Math.min(scaleUp, maximumRunners - currentRunners));
 
     const missingInstanceCount = Math.max(0, scaleUp - newRunners);