feat(scale-down): opt-in idle confirmation window before terminating not-busy runners

lambdas/functions/control-plane/src/aws/runners.d.ts

@@ -21,6 +21,7 @@ export interface RunnerList {
   orphan?: boolean;
   runnerId?: string;
   bypassRemoval?: boolean;
+  idleDetectedAt?: string;
 }
 
 export interface RunnerInfo {

lambdas/functions/control-plane/src/aws/runners.ts

@@ -100,6 +100,7 @@ function getRunnerInfo(runningInstances: DescribeInstancesResult) {
             orphan: i.Tags?.find((e) => e.Key === 'ghr:orphan')?.Value === 'true',
             runnerId: i.Tags?.find((e) => e.Key === 'ghr:github_runner_id')?.Value as string,
             bypassRemoval: i.Tags?.find((e) => e.Key === 'ghr:bypass-removal')?.Value === 'true',
+            idleDetectedAt: i.Tags?.find((e) => e.Key === 'ghr:idle_detected_at')?.Value,
           });
         }
       }

lambdas/functions/control-plane/src/scale-runners/scale-down.test.ts

@@ -826,6 +826,79 @@ describe('Scale down runners', () => {
       expect(runnersTest[2].launchTime).not.toBeDefined();
     });
   });
+
+  describe('With idle confirmation window (SCALE_DOWN_IDLE_CONFIRMATION_SECONDS)', () => {
+    const CONFIRMATION_SECONDS = 300;
+
+    beforeEach(() => {
+      process.env.SCALE_DOWN_IDLE_CONFIRMATION_SECONDS = CONFIRMATION_SECONDS.toString();
+    });
+
+    it('Should start the confirmation window instead of terminating on the first not-busy reading.', async () => {
+      const runners = [createRunnerTestData('idle-1', 'Org', MINIMUM_TIME_RUNNING_IN_MINUTES + 1, true, false, false)];
+      mockGitHubRunners(runners);
+      mockAwsRunners(runners);
+
+      await scaleDown();
+
+      expect(mockTagRunners).toHaveBeenCalledWith(runners[0].instanceId, [
+        { Key: 'ghr:idle_detected_at', Value: expect.any(String) },
+      ]);
+      expect(mockOctokit.actions.deleteSelfHostedRunnerFromOrg).not.toHaveBeenCalled();
+      expect(terminateRunner).not.toHaveBeenCalled();
+    });
+
+    it('Should defer termination while the confirmation window has not elapsed.', async () => {
+      const runners = [createRunnerTestData('idle-1', 'Org', MINIMUM_TIME_RUNNING_IN_MINUTES + 1, true, false, false)];
+      runners[0].idleDetectedAt = new Date(Date.now() - (CONFIRMATION_SECONDS - 240) * 1000).toISOString();
+      mockGitHubRunners(runners);
+      mockAwsRunners(runners);
+
+      await scaleDown();
+
+      expect(mockTagRunners).not.toHaveBeenCalled();
+      expect(mockOctokit.actions.deleteSelfHostedRunnerFromOrg).not.toHaveBeenCalled();
+      expect(terminateRunner).not.toHaveBeenCalled();
+    });
+
+    it('Should terminate once not-busy readings span the confirmation window.', async () => {
+      const runners = [createRunnerTestData('idle-1', 'Org', MINIMUM_TIME_RUNNING_IN_MINUTES + 1, true, false, true)];
+      runners[0].idleDetectedAt = new Date(Date.now() - (CONFIRMATION_SECONDS + 60) * 1000).toISOString();
+      mockGitHubRunners(runners);
+      mockAwsRunners(runners);
+
+      await scaleDown();
+
+      expect(mockOctokit.actions.deleteSelfHostedRunnerFromOrg).toHaveBeenCalled();
+      checkTerminated(runners);
+    });
+
+    it('Should reset the confirmation window when the runner reads busy again.', async () => {
+      const runners = [createRunnerTestData('busy-1', 'Org', MINIMUM_TIME_RUNNING_IN_MINUTES + 1, true, false, false)];
+      runners[0].idleDetectedAt = new Date(Date.now() - (CONFIRMATION_SECONDS + 60) * 1000).toISOString();
+      mockGitHubRunners(runners);
+      mockAwsRunners(runners);
+
+      await scaleDown();
+
+      expect(mockUntagRunners).toHaveBeenCalledWith(runners[0].instanceId, [{ Key: 'ghr:idle_detected_at' }]);
+      expect(terminateRunner).not.toHaveBeenCalled();
+    });
+
+    it('Should restart the confirmation window when the tag value is unparsable.', async () => {
+      const runners = [createRunnerTestData('idle-1', 'Org', MINIMUM_TIME_RUNNING_IN_MINUTES + 1, true, false, false)];
+      runners[0].idleDetectedAt = 'not-a-timestamp';
+      mockGitHubRunners(runners);
+      mockAwsRunners(runners);
+
+      await scaleDown();
+
+      expect(mockTagRunners).toHaveBeenCalledWith(runners[0].instanceId, [
+        { Key: 'ghr:idle_detected_at', Value: expect.any(String) },
+      ]);
+      expect(terminateRunner).not.toHaveBeenCalled();
+    });
+  });
 });
 
 function mockAwsRunners(runners: RunnerTestItem[]) {

lambdas/functions/control-plane/src/scale-runners/scale-down.ts

@@ -161,15 +161,71 @@ async function deleteGitHubRunner(
   }
 }
 
-async function removeRunner(ec2runner: RunnerInfo, ghRunnerIds: number[]): Promise<void> {
+export const IDLE_DETECTED_TAG = 'ghr:idle_detected_at';
+
+function idleConfirmationSeconds(): number {
+  const raw = process.env.SCALE_DOWN_IDLE_CONFIRMATION_SECONDS;
+  const parsed = raw === undefined || raw === '' ? 0 : Number(raw);
+  return Number.isFinite(parsed) && parsed > 0 ? parsed : 0;
+}
+
+// GitHub's busy flag can be stale: it reads false for runners that are actively executing
+// a job, both shortly after job assignment (observed 25-60s lag) and deep into a running
+// job (observed 12+ minutes). See #5085. A single busy=false reading is therefore not
+// sufficient evidence that a runner is idle. When SCALE_DOWN_IDLE_CONFIRMATION_SECONDS > 0,
+// require busy=false readings spanning at least that window before terminating; any
+// busy=true reading in between resets the window (see clearIdleDetection).
+async function idleConfirmed(ec2runner: RunnerInfo): Promise<boolean> {
+  const confirmationSeconds = idleConfirmationSeconds();
+  if (confirmationSeconds === 0) {
+    return true;
+  }
+  const idleDetectedAt = (ec2runner as unknown as RunnerList).idleDetectedAt;
+  const idleForSeconds = idleDetectedAt ? (Date.now() - Date.parse(idleDetectedAt)) / 1000 : NaN;
+  if (Number.isNaN(idleForSeconds)) {
+    // No tag yet, or an unparsable one: (re)start the confirmation window.
+    await tag(ec2runner.instanceId, [{ Key: IDLE_DETECTED_TAG, Value: new Date().toISOString() }]);
+    logger.info(
+      `Runner '${ec2runner.instanceId}' reads idle; deferring termination for at least ` +
+        `${confirmationSeconds}s to confirm the busy state is not stale.`,
+    );
+    return false;
+  }
+  if (idleForSeconds < confirmationSeconds) {
+    logger.info(
+      `Runner '${ec2runner.instanceId}' reads idle since '${idleDetectedAt}' ` +
+        `(${Math.round(idleForSeconds)}s < ${confirmationSeconds}s); deferring termination.`,
+    );
+    return false;
+  }
+  logger.info(
+    `Runner '${ec2runner.instanceId}' confirmed idle since '${idleDetectedAt}' ` +
+      `(${Math.round(idleForSeconds)}s >= ${confirmationSeconds}s).`,
+  );
+  return true;
+}
+
+async function clearIdleDetection(ec2runner: RunnerInfo): Promise<void> {
+  if (idleConfirmationSeconds() === 0) {
+    return;
+  }
+  if ((ec2runner as unknown as RunnerList).idleDetectedAt) {
+    await untag(ec2runner.instanceId, [{ Key: IDLE_DETECTED_TAG }]);
+    logger.info(`Runner '${ec2runner.instanceId}' is busy again; idle-detection window reset.`);
+  }
+}
+
+export type RemoveRunnerResult = 'bypassed' | 'busy' | 'idle-deferred' | 'terminated' | 'delete-failed' | 'error';
+
+async function removeRunner(ec2runner: RunnerInfo, ghRunnerIds: number[]): Promise<RemoveRunnerResult> {
   const githubInstallationClient = await getOrCreateOctokit(ec2runner);
   try {
     const runnerList = ec2runner as unknown as RunnerList;
     if (runnerList.bypassRemoval) {
       logger.info(
         `Runner '${ec2runner.instanceId}' has bypass-removal tag set, skipping removal. Remove the tag to allow scale-down.`,
       );
-      return;
+      return 'bypassed';
     }
 
     const states = await Promise.all(
@@ -180,6 +236,9 @@ async function removeRunner(ec2runner: RunnerInfo, ghRunnerIds: number[]): Promi
     );
 
     if (states.every((busy) => busy === false)) {
+      if (!(await idleConfirmed(ec2runner))) {
+        return 'idle-deferred';
+      }
       const results = await Promise.all(
         ghRunnerIds.map((ghRunnerId) => deleteGitHubRunner(githubInstallationClient, ec2runner, ghRunnerId)),
       );
@@ -190,6 +249,7 @@ async function removeRunner(ec2runner: RunnerInfo, ghRunnerIds: number[]): Promi
       if (allSucceeded) {
         await terminateRunner(ec2runner.instanceId);
         logger.info(`AWS runner instance '${ec2runner.instanceId}' is terminated and GitHub runner is de-registered.`);
+        return 'terminated';
       } else {
         // Only terminate EC2 if we successfully de-registered from GitHub
         // Otherwise, leave the instance running so the next scale-down cycle can retry
@@ -198,15 +258,19 @@ async function removeRunner(ec2runner: RunnerInfo, ghRunnerIds: number[]): Promi
             `Instance will NOT be terminated to allow retry on next scale-down cycle. ` +
             `Failed runner IDs: ${failedRunners.map((r) => r.ghRunnerId).join(', ')}`,
         );
+        return 'delete-failed';
       }
     } else {
+      await clearIdleDetection(ec2runner);
       logger.info(`Runner '${ec2runner.instanceId}' cannot be de-registered, because it is still busy.`);
+      return 'busy';
     }
   } catch (e) {
     logger.error(
       `Runner '${ec2runner.instanceId}' cannot be de-registered. Error: ${e instanceof Error ? e.message : String(e)}`,
       { error: e },
     );
+    return 'error';
   }
 }
 
@@ -217,6 +281,17 @@ async function evaluateAndRemoveRunners(
   let idleCounter = getIdleRunnerCount(scaleDownConfigs);
   const evictionStrategy = getEvictionStrategy(scaleDownConfigs);
   const ownerTags = new Set(ec2Runners.map((runner) => runner.owner));
+  const census: Record<RemoveRunnerResult | 'kept-idle' | 'orphan-marked' | 'not-booted', number> = {
+    bypassed: 0,
+    busy: 0,
+    'idle-deferred': 0,
+    terminated: 0,
+    'delete-failed': 0,
+    error: 0,
+    'kept-idle': 0,
+    'orphan-marked': 0,
+    'not-booted': 0,
+  };
 
   for (const ownerTag of ownerTags) {
     const ec2RunnersFiltered = ec2Runners
@@ -244,21 +319,31 @@ async function evaluateAndRemoveRunners(
           if (idleCounter > 0) {
             idleCounter--;
             logger.info(`Runner '${ec2Runner.instanceId}' will be kept idle.`);
+            census['kept-idle']++;
           } else {
             logger.info(`Terminating all non busy runners.`);
-            await removeRunner(
+            const result = await removeRunner(
               ec2Runner,
               ghRunnersFiltered.map((runner: { id: number }) => runner.id),
             );
+            census[result]++;
           }
         }
       } else if (bootTimeExceeded(ec2Runner)) {
         await markOrphan(ec2Runner.instanceId);
+        census['orphan-marked']++;
       } else {
         logger.debug(`Runner ${ec2Runner.instanceId} has not yet booted.`);
+        census['not-booted']++;
       }
     }
   }
+  logger.info(
+    `Scale-down census: evaluated ${ec2Runners.length} runner(s): ` +
+      Object.entries(census)
+        .map(([key, value]) => `${key}=${value}`)
+        .join(' '),
+  );
 }
 
 async function markOrphan(instanceId: string): Promise<void> {

main.tf

@@ -201,6 +201,7 @@ module "runners" {
   scale_down_schedule_expression       = var.scale_down_schedule_expression
   minimum_running_time_in_minutes      = var.minimum_running_time_in_minutes
   runner_boot_time_in_minutes          = var.runner_boot_time_in_minutes
+  scale_down_idle_confirmation_seconds = var.scale_down_idle_confirmation_seconds
   runner_disable_default_labels        = var.runner_disable_default_labels
   runner_labels                        = local.runner_labels
   runner_as_root                       = var.runner_as_root

modules/multi-runner/runners.tf

@@ -44,6 +44,7 @@ module "runners" {
   scale_down_schedule_expression       = each.value.runner_config.scale_down_schedule_expression
   minimum_running_time_in_minutes      = each.value.runner_config.minimum_running_time_in_minutes
   runner_boot_time_in_minutes          = each.value.runner_config.runner_boot_time_in_minutes
+  scale_down_idle_confirmation_seconds = each.value.runner_config.scale_down_idle_confirmation_seconds
   runner_disable_default_labels        = each.value.runner_config.runner_disable_default_labels
   runner_labels                        = each.value.runner_config.runner_disable_default_labels ? sort(distinct(each.value.runner_config.runner_extra_labels)) : sort(distinct(concat(["self-hosted", each.value.runner_config.runner_os, each.value.runner_config.runner_architecture], each.value.runner_config.runner_extra_labels)))
   runner_as_root                       = each.value.runner_config.runner_as_root

modules/multi-runner/variables.tf

@@ -104,6 +104,7 @@ variable "multi_runner_config" {
       pool_runner_owner                                              = optional(string, null)
       runner_as_root                                                 = optional(bool, false)
       runner_boot_time_in_minutes                                    = optional(number, 5)
+      scale_down_idle_confirmation_seconds                           = optional(number, 0)
       runner_disable_default_labels                                  = optional(bool, false)
       runner_extra_labels                                            = optional(list(string), [])
       runner_group_name                                              = optional(string, "Default")
@@ -249,6 +250,7 @@ variable "multi_runner_config" {
         runner_additional_security_group_ids: "List of additional security groups IDs to apply to the runner. If added outside the multi_runner_config block, the additional security group(s) will be applied to all runner configs. If added inside the multi_runner_config, the additional security group(s) will be applied to the individual runner."
         runner_as_root: "Run the action runner under the root user. Variable `runner_run_as` will be ignored."
         runner_boot_time_in_minutes: "The minimum time for an EC2 runner to boot and register as a runner."
+        scale_down_idle_confirmation_seconds: "Number of seconds a runner must consistently report not-busy before scale-down terminates it. GitHub's busy flag can be stale, so a single not-busy reading is not sufficient evidence a runner is idle. 0 keeps the previous single-reading behaviour."
         runner_disable_default_labels: "Disable default labels for the runners (os, architecture and `self-hosted`). If enabled, the runner will only have the extra labels provided in `runner_extra_labels`. In case you on own start script is used, this configuration parameter needs to be parsed via SSM."
         runner_extra_labels: "Extra (custom) labels for the runners (GitHub). Separate each label by a comma. Labels checks on the webhook can be enforced by setting `multi_runner_config.matcherConfig.exactMatch`. GitHub read-only labels should not be provided."
         runner_group_name: "Name of the runner group."

modules/runners/scale-down.tf

@@ -37,6 +37,7 @@ resource "aws_lambda_function" "scale_down" {
       POWERTOOLS_LOGGER_LOG_EVENT              = var.log_level == "debug" ? "true" : "false"
       RUNNER_BOOT_TIME_IN_MINUTES              = var.runner_boot_time_in_minutes
       SCALE_DOWN_CONFIG                        = jsonencode(var.idle_config)
+      SCALE_DOWN_IDLE_CONFIRMATION_SECONDS     = var.scale_down_idle_confirmation_seconds
       POWERTOOLS_SERVICE_NAME                  = "${var.prefix}-scale-down"
       POWERTOOLS_METRICS_NAMESPACE             = var.metrics.namespace
       POWERTOOLS_TRACE_ENABLED                 = var.tracing_config.mode != null ? true : false

modules/runners/variables.tf

@@ -257,6 +257,12 @@ variable "runner_boot_time_in_minutes" {
   default     = 5
 }
 
+variable "scale_down_idle_confirmation_seconds" {
+  description = "Number of seconds a runner must consistently report not-busy before scale-down terminates it. GitHub's busy flag can be stale (it can read false for a runner that is actively executing a job), so a single not-busy reading is not sufficient evidence a runner is idle. Set to at least one scale-down schedule interval to require two consecutive not-busy evaluations; a busy reading resets the window. 0 keeps the previous single-reading behaviour."
+  type        = number
+  default     = 0
+}
+
 variable "runner_disable_default_labels" {
   description = "Disable default labels for the runners (os, architecture and `self-hosted`). If enabled, the runner will only have the extra labels provided in `runner_extra_labels`."
   type        = bool

variables.tf

@@ -79,6 +79,12 @@ variable "minimum_running_time_in_minutes" {
   default     = null
 }
 
+variable "scale_down_idle_confirmation_seconds" {
+  description = "Number of seconds a runner must consistently report not-busy before scale-down terminates it. GitHub's busy flag can be stale (it can read false for a runner that is actively executing a job), so a single not-busy reading is not sufficient evidence a runner is idle. Set to at least one scale-down schedule interval to require two consecutive not-busy evaluations; a busy reading resets the window. 0 keeps the previous single-reading behaviour."
+  type        = number
+  default     = 0
+}
+
 variable "runner_boot_time_in_minutes" {
   description = "The minimum time for an EC2 runner to boot and register as a runner."
   type        = number