Add windows-pools.ps1 autoscale for the native Windows fleet

[leonarduk]

Closes #112

Summary

• New .\windows-pools.ps1 autoscale [-Once] [-DryRun] [-Interval <s>], driven by a gitignored windows-autoscale.conf (name min max [idle_minutes]). It's separate from autoscale.conf because a pool name can exist in both fleets.
• Same rules as Add pools.sh autoscale: resize pools to their queued jobs #113's pools.sh autoscale. A pool grows to busy + queued, capped at max and never below min. Once idle for idle_minutes, it shrinks to min. A pool is left alone when GitHub can't be asked, and windows-pools.conf is never rewritten.
• Demand: a queued job counts when all its labels are among a slot's own (self-hosted,windows,<arch>,<host>), and only against the first declared Windows pool for that repo. Slots have no extra labels to tell same-repo pools apart.
• Capacity: counts running slots, so a stopped slot is restarted rather than counted.
• Refactor: scale's body is now Invoke-PoolResize, shared with autoscale, and scale's behaviour is unchanged. Assert-RunnersIdle delegates to a new Get-RunnersNotIdleReason, which returns the reason instead of exiting. Get-QueuedJobLabelSets / Invoke-GhJson were added to PoolSlot.ps1.
• Loop: each pass runs in a child process, so one that dies doesn't end the loop.
• windows/README.md "Autoscaling" section and windows-autoscale.conf.example.

Test plan

• Parse check under Windows PowerShell 5.1 (Test-PowerShellParses.ps1): all clean
• tests\windows_pools_test.ps1: 19 new autoscale checks pass under pwsh and 5.1. They cover dry-run, grow, first-pool ownership, linux/other-host/in-progress jobs ignored, idle countdown and state, refused shrink keeping slots and clock, GitHub down, shrink after timeout, conf untouched, raise to min, and skipped conf lines.
• CI PowerShell Tests. Locally on StevesBigLaptop, two existing checks fail: "scale down removes the highest slots and keeps the rest running" fails on unmodified main too, and "Stop-Slot -Force kills the slot process's children too" flips between runs. Both passed in CI for Add pools.sh autoscale: resize pools to their queued jobs #113, so they look like local timing, not this change.
• Dry run on the bedroom PC before enabling the task

🤖 Generated with Claude Code

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: a34b44c6-39b7-4450-b1cb-616add997ff1

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

GPT AI Code Review

Skipped: API key not configured

The OPENAI_API_KEY repository secret is not set.

To enable GPT reviews, add OPENAI_API_KEY as a repository secret:

1. Go to Settings > Secrets and variables > Actions in this repo.
2. Click New repository secret.
3. Name: OPENAI_API_KEY
4. Value: your GPT API key.

Once the secret is configured, re-run this workflow or push a new commit to trigger a fresh review.

REVIEW SKIPPED - API KEY NOT CONFIGURED

---

Reviewed by GPT via gpt-pr-review.yml. Advisory only.

[github-actions]

DeepSeek AI Code Review

1. Acceptance criteria

The diff covers the AC well:

• autoscale command added to windows-pools.ps1 with -Once, -DryRun, -Interval (lines 33-49).
• Demand computed from queued jobs whose labels are a subset of a slot's labels (self-hosted, windows, <arch>, <HostLabel>) — matches the "labels a Windows pool's runners carry" wording.
• Grow to busy + queued capped at max, floor at min — matches.
• Shrink to min after idle_minutes via the existing idle check, refactored into Get-RunnersNotIdleReason so autoscale can carry on rather than exit — matches.
• Counts only, no memory limits — matches.
• windows-autoscale.conf / .example referenced; README updated.

One AC gap worth flagging: the issue says "against windows-pools.conf pools". The implementation reads pool definitions from windows-pools.conf (via Get-PoolConfLine) but the list of pools to autoscale comes from a separate windows-autoscale.conf. That's a reasonable design (mirrors autoscale.conf), and the README explains it, but it's a deviation from a literal reading of the AC. Not blocking given the Linux side does the same thing.

2. Bugs and logic errors

Invoke-PoolResize return-value contract is fragile. The function returns $null on success and a string on refusal. But Start-RunnerPool.ps1 is invoked with | Out-Host, and the function's last statement before return $null is that pipeline. In PowerShell, Out-Host writes to the host, not the pipeline, so this is fine — but the earlier Write-PoolError calls inside the foreach loop also write to host, so they don't leak into the return value either. OK. However, Invoke-PoolResize is called from the scale branch as $why = Invoke-PoolResize ... and then if ($why) { Invoke-PoolRefuse $why }. If Start-RunnerPool.ps1 ever emits anything to the success stream (it currently uses Out-Host, but that's a caller-side guard, not a callee-side guarantee), $why would be truthy and scale would spuriously refuse. Worth a comment or an explicit [void]/$null = on the invocation.

Get-QueuedJobLabelSets pagination is O(runs × jobs) API calls. For each queued/in_progress run it fetches jobs with per_page=100. On a busy repo with many in-progress runs this is a lot of calls per pass, per repo, every Interval seconds. The Linux queued_jobs() presumably does the same, but the README's rate-limit note ("raise --interval if several busy repos are autoscaled") should be echoed in windows/README.md — it currently isn't. Non-blocking but a real operational footgun.

Invoke-GhJson swallows the error body. gh api $Path 2>$null discards stderr, so a 403 (scope) and a 404 (bad path) and a network failure all collapse to $null → "could not ask GitHub". That's the intended conservative behaviour, but it means a misconfigured token scope looks identical to a transient outage and the operator gets no signal. Consider logging $LASTEXITCODE or capturing stderr into the message.

Idle-clock reset on refusal is subtly wrong. In Invoke-AutoscalePass, when Invoke-PoolResize returns a $why, the code does [void]$newState.Add("$($e.Name) $since") — preserving the old $since. But $since was set to $now at the top of the loop iteration unless the pool was already in the idle branch. Look at the flow: $since = $now is the default; it's only overwritten from $oldState inside the if ($target -eq $current -and $queued -eq 0 -and $busy -eq 0 ...) block. So if the pool is idle and past its idle window, $target becomes $e.Min, $reason becomes "idle ...", and $since is still $now (the else branch that would have set $since = $oldState[...] is skipped because the if ($now - $since -ge ...) branch was taken). Then if Invoke-PoolResize refuses, $newState gets "$($e.Name) $now" — resetting the idle clock to now, so the pool waits another full idle_minutes before trying to shrink again. That's arguably the safe behaviour (don't hammer a busy pool), but it's not what the comment "leaving the slots and the idle clock alone" in the test claims, and the test at line ~810 asserts @(Get-Content $asState) -match '^as \d+$' — which passes for any epoch, including a reset one. So the test doesn't actually verify the clock is preserved. Either the code or the test is wrong; the test is definitely too weak.

$owner selection is order-dependent and undocumented in code. $owner = @($declared | Where-Object { $_.Repo -eq $repo })[0].Name — takes the first windows-pools.conf line for the repo. If windows-pools.conf is reordered, which pool absorbs the queued jobs changes. The README documents this ("the first one windows-pools.conf declares") but there's no code comment, and no test that pins the ordering semantics beyond the happy path. Minor.

Get-Content $autoscaleConfPath with no -ErrorAction — if the file exists but is unreadable (permissions), this throws and the whole pass dies. The Test-Path guard at the top of the autoscale branch only checks existence. Low risk on Windows where the file is user-owned, but worth -ErrorAction Stop + try/catch for a clean error.

3. CLI and API safety

• ValidateSet extended with 'autoscale' — additive, no break.
• New params -Once, -DryRun, -Interval are all optional with defaults — no break.
• scale behaviour preserved: the refactor into Invoke-PoolResize keeps the same busy-check semantics, and Invoke-PoolRefuse replaces the inline Assert-RunnersIdle refusal. One behavioural change: previously Assert-RunnersIdle was called unconditionally (it early-returns on -Force and -not $HasSlots); now Invoke-PoolResize calls Get-RunnersNotIdleReason only if (-not $Force). Equivalent. The $LASTEXITCODE handling changed from exit $LASTEXITCODE to throw "Start-RunnerPool.ps1 exited $LASTEXITCODE for $Name" — this changes the exit code path for scale when the child fails. Previously scale exited with the child's code; now it throws, which in a switch block without a trap will surface as a PowerShell error and a non-zero exit, but the specific code is lost. If any caller (CI, wrapper script) depends on the exact exit code from Start-RunnerPool.ps1, this is a subtle break. Worth checking.
• gh calls: Invoke-GhJson uses gh api which inherits the ambient GH_TOKEN/GITHUB_TOKEN — same as the rest of the script. No new scopes needed (actions:read for runs/jobs, administration:read for runners — already required by Get-GhRunnersForRepo). OK.
• No new Python deps, no pyproject.toml change needed.
• Child-process loop: & $shellExe @passArgs with $passArgs built from $HostLabel, $PatFile, $RunnerVersion, $Arch — all passed as separate array elements, so no injection. $scriptPath comes from $MyInvocation.MyCommand.Path, which is the script's own path. Safe.
• $PatFile is passed on the command line to the child process, which means it appears in the process command line (visible to other users via Get-Process/WMI on Windows). The parent already has it in $PatFile; this just widens exposure. Low severity on a single-user runner host, but worth noting.

4. Test coverage

Good coverage of the happy paths and several refusal paths. Gaps:

• The idle-clock-preservation-on-refusal test is ineffective (see §2). It asserts the state file matches ^as \d+$, which is true whether the clock was preserved or reset. This is the one test that would have caught the bug above.
• No test for -Interval loop behaviour (the child-process spawn). Understandable — hard to test — but the $shellExe = (Get-Process -Id $PID).Path trick is untested and is the kind of thing that breaks on constrained hosts.
• No test for Get-QueuedJobLabelSets pagination (the per_page=100 boundary). The fake gh returns a single page.
• No test that scale's exit code on Start-RunnerPool.ps1 failure is unchanged (see §3).
• No test for a pool in windows-autoscale.conf whose repo has no other pool in windows-pools.conf (the $owner lookup would be $null and .Name would throw). Actually — $owner = @($declared | Where-Object { $_.Repo -eq $repo })[0].Name — if $declared has no line for $repo, [0] is $null and .Name on $null in PowerShell 5.1 returns $null (no throw, thanks to the null-conditional-ish behaviour of property access on $null in PS). So $owner is $null, and $e.Name -eq $owner is false for every entry, so $queued stays 0 for all of them. That means a pool whose repo isn't in windows-pools.conf silently never grows. But the earlier Get-PoolConfLine check would have skipped it with "not in windows-pools.conf". So this is unreachable. OK.

5. Suggested follow-up issues

• windows-pools.ps1 scale no longer propagates Start-RunnerPool.ps1's exact exit code after the Invoke-PoolResize refactor.
• Get-QueuedJobLabelSets makes O(runs) API calls per pass; document rate-limit implications in windows/README.md.
• Invoke-GhJson collapses 403/404/network errors into a single "could not ask" message; surface $LASTEXITCODE or stderr.
• Strengthen the idle-clock-preservation test to assert the exact epoch is unchanged, not just that the file matches ^as \d+$.
• $PatFile is passed on the child process command line in the autoscale loop; consider an env var instead.

REQUEST CHANGES — the idle-clock reset on shrink-refusal (§2) is a real behavioural bug: a pool that is refused once will wait a full idle_minutes before retrying, and the test that claims to cover this does not actually verify it. Fix the $since handling (or the test, if the reset is intended) before merge.

---

Reviewed by DeepSeek via deepseek-pr-review.yml. Advisory only.