Fix swallowed error when resolving organization runner group scale targets

[pujitha24]

Motivation:
In getScaleUpTargetWithFunction (controllers/actions.summerwind.net/horizontal_runner_autoscaler_webhook.go),
the outer err variable from an earlier call was shadowed by a new err
declared inside the closure passed to visibleGroups.Traverse. When Traverse
returned a non-nil traverseErr, the function checked traverseErr != nil but
then returned the stale outer err (nil) instead of traverseErr. As a result,
any real error encountered while resolving an organization/enterprise runner
group candidate (e.g. a transient k8s API error) was logged once and then
discarded: the webhook handler treated it as "no matching
HorizontalRunnerAutoscaler for this event", responded 200 OK, and GitHub
recorded a successful delivery with no visible failure anywhere.

This is a plausible contributing cause of reports like the one below, where
webhook-driven autoscaling silently does nothing for one organization while
working for another sharing the same webhook endpoint - a failure in that
org's runner-group resolution would be swallowed exactly this way, leaving no
error trail to diagnose. This is not a complete diagnosis of that report:
other explanations (GitHub App/PAT lacking access to the second org,
mismatched per-org webhook secrets, or --watch-namespace scoping) remain
possible and were not ruled out here.

Approach:
Return the actual traverseErr instead of the shadowed outer err.

Validation:
Added TestGetScaleUpTargetWithFunctionPropagatesTraverseError, which injects
a scaleTarget function that fails for the organization-scoped key and asserts
the error propagates out of getScaleUpTargetWithFunction. Confirmed the test
fails on the pre-fix code (returns nil error instead of the injected one) and
passes after the fix:

go test ./controllers/actions.summerwind.net/... -run TestGetScaleUpTargetWithFunctionPropagatesTraverseError -v

Also ran the full non-envtest test suite for the affected package (all
webhook-related tests pass) and go build ./... and go vet ./controllers/actions.summerwind.net/..., both clean. The Ginkgo/envtest
suite (TestAPIs) in this package requires local etcd/kube-apiserver binaries
that are not available in this environment; it fails identically on
unmodified master, so this is a pre-existing environment limitation unrelated
to this change, not a regression.

Report: #4246
Signed-off-by: Pujitha Paladugu 10557236+pujitha24@users.noreply.github.com

Fixes #4246

[Copilot]

Pull request overview

This pull request fixes an error-propagation bug in the HorizontalRunnerAutoscaler webhook path where an error returned from visibleGroups.Traverse(...) could be inadvertently discarded, causing webhook-driven autoscaling failures to be silently treated as “no matching autoscaler”.

Changes:

• Return traverseErr from getScaleUpTargetWithFunction when traversal fails, instead of returning the stale outer err.
• Add a regression test that injects a failing scaleTarget function and asserts the traversal error propagates to the caller.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
controllers/actions.summerwind.net/horizontal_runner_autoscaler_webhook.go	Fixes swallowed traversal errors by returning traverseErr when Traverse fails.
controllers/actions.summerwind.net/horizontal_runner_autoscaler_webhook_test.go	Adds a regression test covering traversal-error propagation in getScaleUpTargetWithFunction.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[flynnjustin24]

FROM ghcr.io/actions/gha-runner-scale-set-controller:0.13.0

[pujitha24]

This is still green and rebased on master whenever someone has a chance to take a look — happy to adjust anything that would make review easier.