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

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

@@ -38,6 +38,8 @@ const mockComputeProvider = {
   bootTimeExceeded: vi.fn(),
   markOrphan: vi.fn(),
   unmarkOrphan: vi.fn(),
+  markIdle: vi.fn(),
+  unmarkIdle: vi.fn(),
   terminate: vi.fn(),
 } satisfies ScaleDownComputeProvider;
 
@@ -49,6 +51,8 @@ const mockListRunners = vi.mocked(mockComputeProvider.list);
 const mockBootTimeExceeded = vi.mocked(mockComputeProvider.bootTimeExceeded);
 const mockMarkOrphan = vi.mocked(mockComputeProvider.markOrphan);
 const mockUnmarkOrphan = vi.mocked(mockComputeProvider.unmarkOrphan);
+const mockMarkIdle = vi.mocked(mockComputeProvider.markIdle);
+const mockUnmarkIdle = vi.mocked(mockComputeProvider.unmarkIdle);
 const mockTerminateRunners = vi.mocked(mockComputeProvider.terminate);
 
 const cleanEnv = process.env;
@@ -693,6 +697,92 @@ describe('Scale down runners', () => {
   });
 });
 
+describe('Scale down with the idle confirmation window', () => {
+  const CONFIRMATION_SECONDS = 300;
+
+  beforeEach(() => {
+    process.env = { ...cleanEnv };
+    process.env.GITHUB_APP_KEY_BASE64 = 'TEST_CERTIFICATE_DATA';
+    process.env.GITHUB_APP_ID = '1337';
+    process.env.GITHUB_APP_CLIENT_ID = 'TEST_CLIENT_ID';
+    process.env.GITHUB_APP_CLIENT_SECRET = 'TEST_CLIENT_SECRET';
+    process.env.RUNNERS_MAXIMUM_COUNT = '3';
+    process.env.SCALE_DOWN_CONFIG = '[]';
+    process.env.ENVIRONMENT = ENVIRONMENT;
+    process.env.MINIMUM_RUNNING_TIME_IN_MINUTES = MINIMUM_TIME_RUNNING_IN_MINUTES.toString();
+    process.env.RUNNER_BOOT_TIME_IN_MINUTES = MINIMUM_BOOT_TIME.toString();
+    process.env.COMPUTE_PROVIDER_TYPE = mockComputeProvider.type;
+    process.env.SCALE_DOWN_IDLE_CONFIRMATION_SECONDS = CONFIRMATION_SECONDS.toString();
+    vi.clearAllMocks();
+    vi.resetModules();
+    mockedResolveCapability.mockReturnValue(() => mockComputeProvider);
+    mockBootTimeExceeded.mockImplementation((runner) => {
+      return moment(runner.launchTime).add(MINIMUM_BOOT_TIME, 'minutes') < moment(new Date());
+    });
+  });
+
+  it('starts the 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);
+    mockProviderRunners(runners);
+
+    await scaleDown();
+
+    expect(mockMarkIdle).toHaveBeenCalledWith(runners[0].id, expect.any(String));
+    expect(mockTerminateRunners).not.toHaveBeenCalled();
+  });
+
+  it('defers termination while the 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);
+    mockProviderRunners(runners);
+
+    await scaleDown();
+
+    expect(mockMarkIdle).not.toHaveBeenCalled();
+    expect(mockTerminateRunners).not.toHaveBeenCalled();
+  });
+
+  it('terminates once not-busy readings span the 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);
+    mockProviderRunners(runners);
+
+    await scaleDown();
+
+    expect(mockTerminateRunners).toHaveBeenCalledWith(runners[0].id);
+  });
+
+  it('terminates on a single reading when the window is disabled (0)', async () => {
+    process.env.SCALE_DOWN_IDLE_CONFIRMATION_SECONDS = '0';
+    const runners = [createRunnerTestData('idle-1', 'Org', MINIMUM_TIME_RUNNING_IN_MINUTES + 1, true, false, true)];
+    mockGitHubRunners(runners);
+    mockProviderRunners(runners);
+
+    await scaleDown();
+
+    expect(mockMarkIdle).not.toHaveBeenCalled();
+    expect(mockTerminateRunners).toHaveBeenCalledWith(runners[0].id);
+  });
+
+  it('terminates on a single reading when the provider cannot persist idle state', async () => {
+    // A provider that implements neither markIdle nor unmarkIdle must keep the previous
+    // single-reading behaviour rather than deferring forever.
+    const { markIdle: _m, unmarkIdle: _u, ...withoutIdleSupport } = mockComputeProvider;
+    mockedResolveCapability.mockReturnValue(() => withoutIdleSupport as unknown as typeof mockComputeProvider);
+    const runners = [createRunnerTestData('idle-1', 'Org', MINIMUM_TIME_RUNNING_IN_MINUTES + 1, true, false, true)];
+    mockGitHubRunners(runners);
+    mockProviderRunners(runners);
+
+    await scaleDown();
+
+    expect(mockMarkIdle).not.toHaveBeenCalled();
+    expect(mockTerminateRunners).toHaveBeenCalledWith(runners[0].id);
+  });
+});
+
 function mockProviderRunners(runners: RunnerTestItem[]) {
   mockListRunners.mockImplementation(async (_environment, orphan) => {
     return runners.filter((runner) => !orphan || orphan === runner.orphan);

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

@@ -170,6 +170,61 @@ async function deleteGitHubRunner(
   }
 }
 
+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).
+//
+// Providers that cannot persist per-runner state do not implement markIdle/unmarkIdle;
+// for those the window is skipped entirely and behaviour is unchanged.
+async function idleConfirmed(runner: RunnerInfo, computeProvider: ScaleDownComputeProvider): Promise<boolean> {
+  const confirmationSeconds = idleConfirmationSeconds();
+  if (confirmationSeconds === 0 || !computeProvider.markIdle) {
+    return true;
+  }
+  const idleDetectedAt = runner.idleDetectedAt;
+  const idleForSeconds = idleDetectedAt ? (Date.now() - Date.parse(idleDetectedAt)) / 1000 : NaN;
+  if (Number.isNaN(idleForSeconds)) {
+    // No marker yet, or an unparsable one: (re)start the confirmation window.
+    await computeProvider.markIdle(runner.id, new Date().toISOString());
+    logger.info(
+      `Runner '${runner.id}' 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 '${runner.id}' reads idle since '${idleDetectedAt}' ` +
+        `(${Math.round(idleForSeconds)}s < ${confirmationSeconds}s); deferring termination.`,
+    );
+    return false;
+  }
+  logger.info(
+    `Runner '${runner.id}' confirmed idle since '${idleDetectedAt}' ` +
+      `(${Math.round(idleForSeconds)}s >= ${confirmationSeconds}s).`,
+  );
+  return true;
+}
+
+async function clearIdleDetection(runner: RunnerInfo, computeProvider: ScaleDownComputeProvider): Promise<void> {
+  if (idleConfirmationSeconds() === 0 || !computeProvider.unmarkIdle) {
+    return;
+  }
+  if (runner.idleDetectedAt) {
+    await computeProvider.unmarkIdle(runner.id);
+    logger.info(`Runner '${runner.id}' is busy again; idle-detection window reset.`);
+  }
+}
+
 async function removeRunner(
   runner: RunnerInfo,
   ghRunnerIds: number[],
@@ -192,6 +247,9 @@ async function removeRunner(
     );
 
     if (states.every((busy) => busy === false)) {
+      if (!(await idleConfirmed(runner, computeProvider))) {
+        return;
+      }
       const results = await Promise.all(
         ghRunnerIds.map((ghRunnerId) => deleteGitHubRunner(githubInstallationClient, runner, ghRunnerId)),
       );
@@ -213,6 +271,7 @@ async function removeRunner(
         );
       }
     } else {
+      await clearIdleDetection(runner, computeProvider);
       logger.info(`Runner '${runner.id}' cannot be de-registered, because it is still busy.`);
     }
   } catch (e) {

lambdas/libs/compute-providers/aws/ec2/src/control-plane/runners.ts

@@ -100,6 +100,7 @@ function getRunnerInfo(runningInstances: DescribeInstancesResult) {
             orphan: i.Tags?.find((e) => e.Key === 'ghr:orphan')?.Value === 'true',
             githubRunnerId: 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/libs/compute-providers/aws/ec2/src/control-plane/scale-down.ts

@@ -13,12 +13,29 @@ async function unmarkEc2RunnerOrphan(id: string): Promise<void> {
   await untag(id, [{ Key: 'ghr:orphan', Value: 'true' }]);
 }
 
+/**
+ * Idle-confirmation window (see ScaleDownComputeProvider.markIdle). EC2 persists the
+ * observation as an instance tag, so it survives between scale-down invocations without
+ * any extra state store — the same mechanism `ghr:orphan` uses above.
+ */
+export const IDLE_DETECTED_TAG = 'ghr:idle_detected_at';
+
+async function markEc2RunnerIdle(id: string, at: string): Promise<void> {
+  await tag(id, [{ Key: IDLE_DETECTED_TAG, Value: at }]);
+}
+
+async function unmarkEc2RunnerIdle(id: string): Promise<void> {
+  await untag(id, [{ Key: IDLE_DETECTED_TAG }]);
+}
+
 export function createEc2ScaleDownProvider(): Omit<ScaleDownComputeProvider, 'type'> {
   return {
     list: listEc2ScaleDownRunners,
     bootTimeExceeded,
     markOrphan: markEc2RunnerOrphan,
     unmarkOrphan: unmarkEc2RunnerOrphan,
+    markIdle: markEc2RunnerIdle,
+    unmarkIdle: unmarkEc2RunnerIdle,
     terminate: terminateRunner,
   };
 }

lambdas/libs/compute-providers/core/index.ts

@@ -82,6 +82,12 @@ export interface RunnerInfo {
   orphan?: boolean;
   githubRunnerId?: string;
   bypassRemoval?: boolean;
+  /**
+   * When the provider first observed this runner reporting idle, as an ISO-8601 string.
+   * Set and cleared via `markIdle` / `unmarkIdle`; absent when the provider does not
+   * implement the idle-confirmation window.
+   */
+  idleDetectedAt?: string;
 }
 
 export interface ListRunnerFilters {
@@ -97,6 +103,17 @@ export interface ScaleDownComputeProvider extends ComputeProvider {
   markOrphan(id: string): Promise<void>;
   unmarkOrphan(id: string): Promise<void>;
   terminate(id: string): Promise<void>;
+  /**
+   * Record that the runner was observed idle at `at` (ISO-8601), so a later cycle can tell
+   * how long it has read idle. Surfaces back on `RunnerInfo.idleDetectedAt`.
+   *
+   * OPTIONAL on purpose: a provider with nowhere to persist per-runner state stays valid
+   * against this interface, and scale-down simply skips the confirmation window for it
+   * rather than failing. Only providers implementing BOTH halves get the behaviour.
+   */
+  markIdle?(id: string, at: string): Promise<void>;
+  /** Clear the idle marker — the runner was seen busy again, so the window restarts. */
+  unmarkIdle?(id: string): Promise<void>;
 }
 
 export interface RunnerStatus {

lambdas/libs/compute-providers/templates/provider/control-plane.ts

@@ -67,6 +67,11 @@ export function createTemplateScaleDownProvider(): Omit<ScaleDownComputeProvider
     },
     markOrphan: async (id) => notImplemented(`scaleDown.markOrphan(${id})`),
     unmarkOrphan: async (id) => notImplemented(`scaleDown.unmarkOrphan(${id})`),
+    // Optional. Implement BOTH to opt into the scale-down idle-confirmation window
+    // (SCALE_DOWN_IDLE_CONFIRMATION_SECONDS); omit both if the provider has nowhere to
+    // persist per-runner state, and scale-down keeps its single-reading behaviour.
+    markIdle: async (id, at) => notImplemented(`scaleDown.markIdle(${id}, ${at})`),
+    unmarkIdle: async (id) => notImplemented(`scaleDown.unmarkIdle(${id})`),
     terminate: async (id) => notImplemented(`scaleDown.terminate(${id})`),
   };
 }

main.tf

@@ -215,6 +215,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

@@ -136,6 +136,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")
@@ -281,6 +282,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

@@ -38,6 +38,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

@@ -270,6 +270,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

@@ -100,6 +100,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