Skip to content

Commit 40ab44a

Browse files
authored
fix: don't refuse warm-machine commands in CI or advertise absent Lab (#8342)
The resource-policy warm-machine guard refused non-interactive commands inside GitHub Actions, failing otherwise-good PR checks with no human to rerun and no Lab runner to route to (#7735). Bypass the non-interactive refusal when GITHUB_ACTIONS=true via a new is_ci_execution() helper. The same guard also recommended connecting/using a Lab runner even on hosts with no Lab configured, steering agents toward nonexistent infrastructure (#7749). Make the no-runner portable warning and refusal state honestly that Lab offload is unavailable and offer only applicable actions (defer to CI, connect a runner, explicit local). Refactor primary_action to branch on runner/portability facts instead of fragile message substring matching. Fixes #7735 Fixes #7749
1 parent 1a435db commit 40ab44a

2 files changed

Lines changed: 111 additions & 11 deletions

File tree

src/commands/utils/resource_policy.rs

Lines changed: 96 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,9 @@ use crate::commands::resources::{DoctorOutput, ResourceRecommendation};
1212
pub use crate::core::resource_policy_context::reset_captured_context_for_test;
1313
pub use crate::core::resource_policy_context::{
1414
capture_context, captured_context, clear_managed_runner_placement_context,
15-
clear_runner_hosted_exec, is_managed_runner_placement_context, is_runner_hosted_exec,
16-
ResourcePolicyContext, ResourcePolicyHostSnapshot, ResourcePolicyRunnerSelection,
15+
clear_runner_hosted_exec, is_ci_execution, is_managed_runner_placement_context,
16+
is_runner_hosted_exec, ResourcePolicyContext, ResourcePolicyHostSnapshot,
17+
ResourcePolicyRunnerSelection,
1718
};
1819

1920
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -205,7 +206,11 @@ pub fn non_interactive_preflight_error(
205206
local_hot_rerun_command: Option<String>,
206207
default_runner: Option<&str>,
207208
) -> Option<crate::core::Error> {
208-
if local_override || interactive || is_runner_hosted_exec() {
209+
// GitHub Actions runners are ephemeral, single-purpose, and always
210+
// non-interactive: the warm-machine refusal would fail otherwise-good PR
211+
// checks with no human to rerun and no Lab runner to route to. Never refuse
212+
// inside CI (#7735).
213+
if local_override || interactive || is_runner_hosted_exec() || is_ci_execution() {
209214
return None;
210215
}
211216
if default_runner.is_some() && warning.message.contains("--runner") {
@@ -268,16 +273,25 @@ fn append_local_placement(rerun: &mut Vec<String>, args: &[String]) {
268273
}
269274

270275
fn primary_action(warning: &ResourcePolicyWarning, default_runner: Option<&str>) -> String {
271-
if warning.message.contains("--runner <id>") {
272-
return "No eligible Homeboy Lab runner was found. Connect a runner or pass --runner <id> to offload this portable command; use the local-hot rerun command only as a last resort.".to_string();
273-
}
276+
// A portable command's warning names either a concrete default runner
277+
// ("Lab runner `<id>`") or, when none is configured, states Lab offload is
278+
// unavailable. Local-only commands say neither. Branch on those facts rather
279+
// than inferring Lab availability from placeholder substrings.
274280
if let Some(runner_id) = default_runner {
275-
if warning.message.contains("--runner") {
281+
if warning.message.contains(&format!("`{runner_id}`")) {
276282
return format!(
277283
"Homeboy found Lab runner `{runner_id}`; rerun with --runner {runner_id} or let automatic Lab routing handle this portable command."
278284
);
279285
}
280286
}
287+
if warning
288+
.message
289+
.contains("Lab offload is not currently available")
290+
{
291+
// Portable command, but no Lab runner is configured on this host (#7749):
292+
// do not point the operator at nonexistent Lab infrastructure.
293+
return "No Homeboy Lab runner is configured on this host, so Lab offload is not available. Defer verification to CI, connect a runner with `homeboy runner connect`, or use the local-hot rerun command only if local execution is explicitly authorized.".to_string();
294+
}
281295
if warning.message.contains("--runner") {
282296
"Pass --runner <id> when Lab offload supports this mode.".to_string()
283297
} else {
@@ -301,7 +315,7 @@ fn warning_message(
301315
);
302316
}
303317
return format!(
304-
"Resource policy warning: machine is {severity}; starting `{}` may skew results or add pressure. {reason} Connect a default Homeboy Lab runner or use --runner <id> to route this portable command through Lab offload, or use --placement local to run locally without this warning.",
318+
"Resource policy warning: machine is {severity}; starting `{}` may skew results or add pressure. {reason} No Homeboy Lab runner is configured on this host, so Lab offload is not currently available; connect a runner (`homeboy runner connect`) to enable it, defer verification to CI, or use --placement local to run locally without this warning.",
305319
command.label
306320
);
307321
}
@@ -419,7 +433,9 @@ mod tests {
419433
assert_eq!(warning.command, "bench");
420434
assert_eq!(warning.recommendation, ResourceRecommendation::Hot);
421435
assert!(warning.message.contains("--placement local"));
422-
assert!(warning.message.contains("--runner <id>"));
436+
assert!(warning
437+
.message
438+
.contains("No Homeboy Lab runner is configured on this host"));
423439
assert!(warning.message.contains("Load average is 9.0"));
424440

425441
assert!(evaluate(
@@ -551,7 +567,9 @@ mod tests {
551567
assert_eq!(error.code.as_str(), "validation.invalid_argument");
552568
assert!(error.message.contains("Refusing to start `audit`"));
553569
assert!(error.message.contains("non-interactive shell"));
554-
assert!(error.message.contains("--runner <id>"));
570+
assert!(error
571+
.message
572+
.contains("No Homeboy Lab runner is configured on this host"));
555573
assert!(error.details.get("rerun_command").is_none());
556574
}
557575

@@ -646,7 +664,12 @@ mod tests {
646664
);
647665
assert!(error
648666
.message
649-
.contains("No eligible Homeboy Lab runner was found"));
667+
.contains("No Homeboy Lab runner is configured on this host"));
668+
// #7749: the refusal must not point the operator at Lab infrastructure
669+
// that does not exist on this host.
670+
assert!(!error
671+
.message
672+
.contains("Connect a default Homeboy Lab runner"));
650673
}
651674

652675
#[test]
@@ -804,6 +827,68 @@ mod tests {
804827
assert_eq!(value["host"]["cpu_count"], 4);
805828
}
806829

830+
#[test]
831+
fn ci_execution_does_not_fail_non_interactive_preflight() {
832+
// #7735: inside GitHub Actions the warm-machine refusal must not fire.
833+
// The runner is ephemeral and non-interactive by design; refusing there
834+
// fails otherwise-good PR checks.
835+
let _lock = env_lock();
836+
let _hosted = EnvVarGuard::remove(crate::core::runner::RUNNER_HOSTED_EXEC_ENV);
837+
let _ci = EnvVarGuard::set("GITHUB_ACTIONS", "true");
838+
let warning = evaluate(
839+
lab_supported_hot("review test"),
840+
&resources(ResourceRecommendation::Hot),
841+
)
842+
.expect("hot machines warn");
843+
844+
assert!(non_interactive_preflight_error(&warning, false, false, None, None).is_none());
845+
}
846+
847+
#[test]
848+
fn non_ci_shell_still_refuses_when_warm() {
849+
// Guard against the CI bypass leaking into ordinary non-interactive
850+
// shells (e.g. cron, agent runners) where the refusal is still correct.
851+
let _lock = env_lock();
852+
let _hosted = EnvVarGuard::remove(crate::core::runner::RUNNER_HOSTED_EXEC_ENV);
853+
let _ci = EnvVarGuard::remove("GITHUB_ACTIONS");
854+
let warning = evaluate(
855+
lab_supported_hot("review test"),
856+
&resources(ResourceRecommendation::Hot),
857+
)
858+
.expect("hot machines warn");
859+
860+
assert!(non_interactive_preflight_error(&warning, false, false, None, None).is_some());
861+
}
862+
863+
#[test]
864+
fn portable_warning_without_runner_does_not_advertise_lab() {
865+
// #7749: on a host with no Lab runner configured, the warning must not
866+
// recommend connecting/using a Lab runner as if one were available.
867+
let warning = evaluate(
868+
lab_supported_hot("review test"),
869+
&resources(ResourceRecommendation::Hot),
870+
)
871+
.expect("hot machines warn");
872+
873+
assert!(warning
874+
.message
875+
.contains("No Homeboy Lab runner is configured on this host"));
876+
assert!(warning
877+
.message
878+
.contains("Lab offload is not currently available"));
879+
assert!(!warning
880+
.message
881+
.contains("Connect a default Homeboy Lab runner"));
882+
// A genuinely configured runner should still be named.
883+
let with_runner = evaluate_with_runner_hint(
884+
lab_supported_hot("review test"),
885+
&resources(ResourceRecommendation::Hot),
886+
Some("homeboy-lab"),
887+
)
888+
.expect("hot machines warn");
889+
assert!(with_runner.message.contains("Lab runner `homeboy-lab`"));
890+
}
891+
807892
struct EnvVarGuard {
808893
name: &'static str,
809894
prior: Option<String>,

src/core/resource_policy_context.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,21 @@ pub fn is_runner_hosted_exec() -> bool {
123123
.is_some_and(|value| value == "1")
124124
}
125125

126+
/// True when Homeboy is executing inside a GitHub Actions CI job.
127+
///
128+
/// CI runners are ephemeral, single-purpose, and non-interactive by design.
129+
/// The resource-policy warm-machine refusal exists to protect a shared
130+
/// developer/controller host from added load and to steer work onto Lab; inside
131+
/// CI there is no shared host to protect, no human to "rerun later", and no Lab
132+
/// runner to route to. Refusing there turns the guard itself into the outage
133+
/// (the PR check goes red on good code), so callers use this to bypass or
134+
/// downgrade the non-interactive refusal in CI.
135+
pub fn is_ci_execution() -> bool {
136+
std::env::var("GITHUB_ACTIONS")
137+
.ok()
138+
.is_some_and(|value| value == "true")
139+
}
140+
126141
/// True for the complete environment prepared by a managed remote runner exec.
127142
///
128143
/// Environment variables are not cryptographic provenance: a process with

0 commit comments

Comments
 (0)