diff --git a/crates/preloop-cli/build.rs b/crates/preloop-cli/build.rs index 561bf1e5..613fc6f4 100644 --- a/crates/preloop-cli/build.rs +++ b/crates/preloop-cli/build.rs @@ -11,6 +11,7 @@ use std::env; use std::fs; use std::path::PathBuf; +use std::process::Command; fn main() { let manifest_dir = PathBuf::from(env::var("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR")); @@ -43,4 +44,52 @@ fn main() { let out_dir = PathBuf::from(env::var("OUT_DIR").expect("OUT_DIR")); fs::write(out_dir.join("pins.rs"), out).expect("write pins.rs"); println!("cargo:rerun-if-changed=../../versions.toml"); + + // Embed the exact source commit this binary was built from. The updater + // uses it to resolve same-version installs: a source build from newer + // main reports the same version string as the latest release tag, and + // byte-comparing the binaries would clobber the newer build with the + // stale release asset. The commit is the monotonic signal the version + // string cannot carry. Release assets are built in CI from a git + // checkout, so the SHA is always present there; a non-git build (e.g. + // `cargo install` from a crates.io tarball) falls back to a sentinel + // that the updater treats as "cannot verify, keep what is installed". + let commit = Command::new("git") + .args(["rev-parse", "HEAD"]) + .current_dir(&manifest_dir) + .output() + .ok() + .filter(|output| output.status.success()) + .and_then(|output| String::from_utf8(output.stdout).ok()) + .map(|sha| sha.trim().to_owned()) + .filter(|sha| !sha.is_empty()) + .unwrap_or_else(|| "unknown".to_owned()); + println!("cargo:rustc-env=PRELOOP_BUILD_COMMIT={commit}"); + // Make Cargo rebuild this crate when the checked-out commit changes. + // `.git/HEAD` alone is not enough: on a branch checkout it contains the + // symbolic ref (`ref: refs/heads/main`) whose contents do not change + // when the branch advances, so an incremental build would keep embedding + // the previous commit — and the updater could then compare that stale + // commit and clobber a newer source build with the release binary. + // Track HEAD, the ref it points at, and packed-refs (the ref file may + // live there instead of under refs/), all resolved through git itself. + let mut ref_paths = vec![String::from("../../.git/HEAD")]; + if let Ok(output) = Command::new("git") + .args(["rev-parse", "--symbolic-full-name", "HEAD"]) + .current_dir(&manifest_dir) + .output() + { + if output.status.success() { + if let Ok(ref_name) = String::from_utf8(output.stdout) { + let ref_name = ref_name.trim(); + if let Some(short) = ref_name.strip_prefix("refs/") { + ref_paths.push(format!("../../.git/{short}")); + } + } + } + } + ref_paths.push(String::from("../../.git/packed-refs")); + for path in ref_paths { + println!("cargo:rerun-if-changed={path}"); + } } diff --git a/crates/preloop-cli/src/main.rs b/crates/preloop-cli/src/main.rs index 3d00e702..2acf66b0 100644 --- a/crates/preloop-cli/src/main.rs +++ b/crates/preloop-cli/src/main.rs @@ -764,7 +764,11 @@ async fn main() -> anyhow::Result<()> { // one underneath itself. match cli.command { Command::Version => { - println!("preloop {}", env!("CARGO_PKG_VERSION")); + println!( + "preloop {} ({})", + env!("CARGO_PKG_VERSION"), + env!("PRELOOP_BUILD_COMMIT") + ); return Ok(()); } Command::Serve(args) => return cmd_engine(args).await, diff --git a/crates/preloop-cli/src/push.rs b/crates/preloop-cli/src/push.rs index 5f093d94..3127edc0 100644 --- a/crates/preloop-cli/src/push.rs +++ b/crates/preloop-cli/src/push.rs @@ -581,6 +581,12 @@ mod tests { b.path().to_str().unwrap(), ], ); + // The commit happens in B, and a clone does not inherit A's local + // config. Without this, the test only passes on hosts whose global + // git identity leaks into B (the CI fork has none, so it failed with + // "Author identity unknown"). + git(b.path(), &["config", "user.email", "test@example.com"]); + git(b.path(), &["config", "user.name", "Test"]); assert_eq!( git(b.path(), &["rev-parse", "HEAD"]), head, diff --git a/crates/preloop-cli/src/update.rs b/crates/preloop-cli/src/update.rs index 48078bdd..5176275a 100644 --- a/crates/preloop-cli/src/update.rs +++ b/crates/preloop-cli/src/update.rs @@ -153,20 +153,25 @@ pub(crate) async fn run(args: UpdateArgs) -> anyhow::Result<()> { None => bail!("release {} has no asset for {target}", release.tag_name), }; if remote_version == current_version { - // The version string is self-reported and can lie: a source build or - // a tampered binary claims the release version while its bytes - // differ, and a version-only gate then declares it up to date - // forever (this is how the v0.30.2 deaf-runner fix never reached - // production). Verify the installed binary against the checksummed - // release asset and reinstall on mismatch. - match check_same_version_content(&client, &selected).await { - Ok(ContentCheck::Matches) => { + // The version string is self-reported and can lie: a source build + // claims the release version while it predates the release commit, + // and a version-only gate then declares it up to date forever (this + // is how the v0.30.2 deaf-runner fix never reached production). But + // byte-comparing against the release asset has the opposite failure: + // a source build from *newer* main also reports the same version + // (no bump between the tag and HEAD) and would be clobbered with the + // stale release binary. The commit is the monotonic signal the + // version string cannot carry: reinstall only when the release + // commit is an ancestor of the installed one (the release is + // strictly newer); keep anything newer or unverifiable. + match resolve_same_version(&client, &api_url, &release.tag_name).await { + Ok(SameVersionDecision::UpToDate) => { println!("preloop {} is already up to date", current_version); return Ok(()); } - Ok(ContentCheck::Drift(staged)) => { + Ok(SameVersionDecision::Reinstall) => { println!( - "preloop {} does not match release {}; {} ({target})", + "preloop {} is older than release {}; {} ({target})", current_version, release.tag_name, if args.check { @@ -178,6 +183,7 @@ pub(crate) async fn run(args: UpdateArgs) -> anyhow::Result<()> { if args.check { return Ok(()); } + let staged = stage_release(&client, &selected).await?; let lock_path = update_lock_path()?; let _lock = UpdateLock::acquire(&lock_path)?; let executable = @@ -190,7 +196,9 @@ pub(crate) async fn run(args: UpdateArgs) -> anyhow::Result<()> { } Err(error) => { // A transient failure to fetch or verify the asset must not - // fail the hourly update timer; the next run retries. + // fail the hourly update timer; the next run retries. Keep + // what is installed: an unverifiable comparison must never + // clobber a possibly-newer build. println!( "warning: could not verify the installed binary against release {}: {error:#}", release.tag_name @@ -722,37 +730,100 @@ async fn stage_release( }) } -enum ContentCheck { - Matches, - Drift(StagedRelease), +enum SameVersionDecision { + /// Installed binary is the same commit as, or newer than, the release. + UpToDate, + /// The release commit is an ancestor of the installed one; reinstall. + /// The release binary is staged by the caller after the `--check` guard, + /// so check-only runs never download the asset. + Reinstall, } -/// Compare the installed binary against the checksummed release asset. -async fn check_same_version_content( +/// Resolve an equal-version install by comparing the commit the installed +/// binary was built from against the release tag's commit. +/// +/// The version string is the only thing the two share, so it cannot decide +/// this case. The embedded build commit is monotonic: a source build from +/// newer main is a *descendant* of the release commit (keep it — it carries +/// fixes the release does not), while a source build that predates the +/// release tag is an *ancestor* (reinstall — this is the v0.30.2 deaf-runner +/// case where the fix never reached production). Anything unverifiable +/// (no embedded commit, diverged history, compare API failure) is kept: +/// clobbering a possibly-newer build on a heuristic is worse than one +/// extra hourly poll. +async fn resolve_same_version( client: &Client, - selected: &SelectedAsset<'_>, -) -> anyhow::Result { - let staged = stage_release(client, selected).await?; - - let installed = std::env::current_exe().context("locate running preloop executable")?; - // macOS installs are launched through the `preloop` symlink into - // `/bin/preloop`; canonicalize so a future compare of paths - // (and anyone reading this) sees the real file. - let installed = - fs::canonicalize(&installed).with_context(|| format!("resolve {}", installed.display()))?; - if installed_binary_matches(&installed, &staged.binary_path)? { - Ok(ContentCheck::Matches) - } else { - Ok(ContentCheck::Drift(staged)) + api_url: &str, + release_tag: &str, +) -> anyhow::Result { + let installed_commit = env!("PRELOOP_BUILD_COMMIT"); + if installed_commit == "unknown" { + println!( + "installed binary has no embedded build commit; keeping it (cannot verify against {} {release_tag})", + env!("CARGO_PKG_VERSION") + ); + return Ok(SameVersionDecision::UpToDate); + } + + let url = compare_url(api_url, installed_commit, release_tag); + let response = client + .get(&url) + .send() + .await + .with_context(|| format!("poll GitHub compare API: {url}"))?; + let status = response.status(); + if !status.is_success() { + bail!("GitHub compare API returned {status} for {url}"); + } + #[derive(Deserialize)] + struct Compare { + status: String, + } + let compare: Compare = response + .json() + .await + .with_context(|| format!("decode compare response from {url}"))?; + match same_version_decision(compare.status.as_str()) { + SameVersionOutcome::ReleaseNewer => Ok(SameVersionDecision::Reinstall), + SameVersionOutcome::KeepInstalled => Ok(SameVersionDecision::UpToDate), } } -/// Content comparison behind the same-version check: `true` only when the -/// installed binary is byte-identical to the release binary. A missing or -/// unreadable file is an `Err`, never a silent `true` — the caller treats -/// "unknown" as "keep what is installed and retry later", not "matches". -fn installed_binary_matches(installed: &Path, release_binary: &Path) -> anyhow::Result { - Ok(sha256_file(installed)? == sha256_file(release_binary)?) +/// Build the compare API URL from the configured releases API base. +/// +/// `/releases` (the endpoint `fetch_release` polls) becomes +/// `/compare/{installed}...{release}`. Deriving from the configured +/// base keeps GitHub Enterprise and `PRELOOP_RELEASES_API` overrides working +/// — a hard-coded api.github.com host would fail the comparison for those +/// setups and never reinstall an older build. +fn compare_url(api_url: &str, installed_commit: &str, release_tag: &str) -> String { + let base = api_url + .trim_end_matches('/') + .strip_suffix("/releases") + .unwrap_or(api_url.trim_end_matches('/')); + format!("{base}/compare/{installed_commit}...{release_tag}") +} + +/// Map the GitHub compare API's `status` to a same-version decision. +/// +/// The endpoint reports from the perspective of `head` (the release tag): +/// `ahead` means the release is strictly ahead of the installed commit — +/// reinstall. `behind`/`identical` mean the installed commit is at or +/// beyond the release — keep. Anything else (a `diverged` history, or an +/// unexpected status) keeps what is installed: never clobber a real build +/// over an ambiguous comparison. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum SameVersionOutcome { + ReleaseNewer, + KeepInstalled, +} + +fn same_version_decision(compare_status: &str) -> SameVersionOutcome { + match compare_status { + "ahead" => SameVersionOutcome::ReleaseNewer, + "behind" | "identical" => SameVersionOutcome::KeepInstalled, + _ => SameVersionOutcome::KeepInstalled, + } } fn extract_binary(archive_path: &Path, destination: &Path) -> anyhow::Result<()> { @@ -995,6 +1066,70 @@ mod tests { ))); } + /// The release commit is strictly ahead of the installed commit (a + /// source build older than the tag — the v0.30.2 deaf-runner case): + /// reinstall so the fix that never reached production finally does. + #[test] + fn same_version_release_ahead_reinstalls() { + assert_eq!( + same_version_decision("ahead"), + SameVersionOutcome::ReleaseNewer + ); + } + + /// Installed commit is at or beyond the release (a source build from + /// newer main with no version bump — the production regression this + /// replaces the byte compare for): keep the newer build. + #[test] + fn same_version_installed_at_or_ahead_keeps() { + assert_eq!( + same_version_decision("behind"), + SameVersionOutcome::KeepInstalled + ); + assert_eq!( + same_version_decision("identical"), + SameVersionOutcome::KeepInstalled + ); + } + + /// A diverged history (or any unexpected status) must keep what is + /// installed: clobbering a real build over an ambiguous comparison is + /// the failure mode we are removing. + #[test] + fn same_version_diverged_or_unknown_keeps() { + for status in ["diverged", "unknown", "unexpected"] { + assert_eq!( + same_version_decision(status), + SameVersionOutcome::KeepInstalled, + "status {status:?} must keep installed" + ); + } + } + + /// The compare endpoint must derive from the configured releases API + /// base, not a hard-coded github.com host — a GitHub Enterprise or + /// overridden endpoint would otherwise always fail the comparison and + /// never reinstall an older build. + #[test] + fn compare_url_derives_from_the_configured_api_base() { + for (api_url, expected_base) in [ + ( + "https://api.github.com/repos/preloopdev/preloop/releases", + "https://api.github.com/repos/preloopdev/preloop/compare", + ), + ( + "https://github.example.com/api/v3/repos/acme/preloop/releases", + "https://github.example.com/api/v3/repos/acme/preloop/compare", + ), + ] { + assert_eq!( + compare_url(api_url, "deadbeef", "v0.30.10"), + format!("{expected_base}/deadbeef...v0.30.10"), + "compare URL must use the configured API base: {api_url}" + ); + } + } + #[cfg(unix)] #[tokio::test] async fn probe_detects_mount_socket_in_help_text() { @@ -1362,74 +1497,6 @@ mod tests { assert_eq!(std::fs::read(output_path).expect("binary"), contents); } - #[test] - fn content_check_detects_same_version_drift() { - // The v0.30.2 incident: a locally built binary claimed the release - // version string, so the version-only gate declared it up to date - // and the shipped fix never installed. The same-version check must - // compare bytes, not versions: identical content matches, drifted - // content (same claimed version) does not, and an unreadable file is - // an error rather than a silent match. - let temp = tempfile::tempdir().expect("staging directory"); - let installed = temp.path().join("installed"); - let release = temp.path().join("release"); - std::fs::write(&installed, b"installed-build").unwrap(); - std::fs::write(&release, b"installed-build").unwrap(); - assert!( - installed_binary_matches(&installed, &release).expect("both files readable"), - "byte-identical binaries must match" - ); - std::fs::write(&release, b"release-build").unwrap(); - assert!( - !installed_binary_matches(&installed, &release).expect("both files readable"), - "drifted content at the same version must be detected" - ); - assert!( - installed_binary_matches(&installed, &temp.path().join("missing")).is_err(), - "an unreadable binary must not be reported as matching" - ); - } - - #[test] - fn extract_then_content_check_rejects_a_tampered_archive_payload() { - use flate2::write::GzEncoder; - use flate2::Compression; - - let temp = tempfile::tempdir().expect("staging directory"); - let archive_path = temp.path().join("preloop-cli-aarch64-apple-darwin.tar.gz"); - let file = std::fs::File::create(&archive_path).expect("archive"); - let encoder = GzEncoder::new(file, Compression::default()); - let mut builder = tar::Builder::new(encoder); - // Same archive layout, different payload bytes than the "installed" - // binary that claims the same version. - let payload = b"drifted-release-payload"; - let mut header = tar::Header::new_gnu(); - header.set_size(payload.len() as u64); - header.set_mode(0o755); - header.set_cksum(); - builder - .append_data( - &mut header, - "preloop-cli-aarch64-apple-darwin/preloop", - &payload[..], - ) - .expect("binary entry"); - builder - .into_inner() - .expect("gzip stream") - .finish() - .expect("archive"); - - let installed = temp.path().join("installed"); - std::fs::write(&installed, b"local-build-claiming-same-version").unwrap(); - let extracted = temp.path().join(binary_name()); - extract_binary(&archive_path, &extracted).expect("extract binary"); - assert!( - !installed_binary_matches(&installed, &extracted).expect("both files readable"), - "a drifted payload at the same version must trigger reinstall" - ); - } - #[test] fn safe_asset_filename_rejects_traversal_and_absolute_paths() { assert!(safe_asset_filename("preloop-v0.30.2-aarch64-apple-darwin.tar.gz").is_ok()); diff --git a/crates/preloop-orchestrator/src/lib.rs b/crates/preloop-orchestrator/src/lib.rs index 3c4cfd2f..06b5046e 100644 --- a/crates/preloop-orchestrator/src/lib.rs +++ b/crates/preloop-orchestrator/src/lib.rs @@ -264,6 +264,62 @@ fn relax_externals_permissions(externals: &Path) { #[cfg(not(unix))] fn relax_externals_permissions(_externals: &Path) {} +/// Total physical memory in MiB, or `None` when it cannot be determined. +/// +/// Used to bound on-demand fork concurrency so the pool never schedules +/// more runner VMs than the host can hold in RAM. `None` (an unreadable +/// `/proc/meminfo`, a non-Linux/non-macOS host) falls back to CPU-only +/// sizing rather than refusing to run. +#[cfg(target_os = "linux")] +fn host_memory_mib() -> Option { + let meminfo = std::fs::read_to_string("/proc/meminfo").ok()?; + let kib: u64 = meminfo.lines().find_map(|line| { + let line = line.trim(); + let rest = line.strip_prefix("MemTotal:")?; + rest.trim().strip_suffix(" kB")?.trim().parse().ok() + })?; + Some(kib / 1024) +} + +/// Total physical memory in MiB, or `None` when it cannot be determined. +#[cfg(target_os = "macos")] +fn host_memory_mib() -> Option { + let output = std::process::Command::new("sysctl") + .args(["-n", "hw.memsize"]) + .output() + .ok()?; + let bytes: u64 = String::from_utf8_lossy(&output.stdout) + .trim() + .parse() + .ok()?; + Some(bytes / (1024 * 1024)) +} + +/// Total physical memory in MiB, or `None` when it cannot be determined. +#[cfg(not(any(target_os = "linux", target_os = "macos")))] +fn host_memory_mib() -> Option { + None +} + +/// On-demand fork concurrency allowed by host memory alone. +/// +/// Every on-demand fork inherits the golden's committed footprint and grows +/// toward `runner_memory_mib` as its guest runs, so the pool must never +/// schedule more concurrent runners than `(host_total - golden - reserve) / +/// runner_ceiling` allows. The 2 GiB reserve keeps the control plane, OS, +/// and page cache alive — without it the host OOMs *after* the forks are up, +/// which is exactly the production failure this guards against. Floors at 1 +/// so a tiny host still runs a single job rather than refusing to work. +fn on_demand_memory_cap(host_total_mib: u64, runner_memory_mib: u64) -> usize { + const HOST_RESERVE_MIB: u64 = 2048; + // Both the golden and each runner use the same ceiling. A degenerate + // zero ceiling means "unbounded" would be unsafe to divide by, so treat + // it as 1; the config layer validates `memory_mib > 0` in practice. + let runner_mib = runner_memory_mib.max(1); + let golden_mib = runner_mib; + (host_total_mib.saturating_sub(golden_mib + HOST_RESERVE_MIB) / runner_mib).max(1) as usize +} + fn default_golden_url(release_version: &str) -> String { format!( "https://github.com/preloopdev/preloop/releases/download/v{release_version}/preloop-ubuntu-24.04-{}", @@ -1362,6 +1418,15 @@ const GUEST_READY_TIMEOUT: Duration = Duration::from_secs(30); /// golden fork base. Bounded retries; the probe loop is exercised by tests /// under paused Tokio time, so this is the only knob the delay is tied to. const GOLDEN_DRAIN_PROBE_DELAY: Duration = Duration::from_secs(10); +/// Ceiling for the drain-probe backoff: probes start at +/// `GOLDEN_DRAIN_PROBE_DELAY` and double up to this cap, so a long-running +/// job's clone is re-checked every minute rather than every ten seconds. +const GOLDEN_DRAIN_PROBE_MAX: Duration = Duration::from_secs(60); +/// Total wall-clock budget for waiting on live clones to drain before +/// falling back to independent OCI creation. The fork path is orders of +/// magnitude faster than direct creation, so this is generous; a clone that +/// has not exited within it is unlikely to do so soon. +const GOLDEN_DRAIN_BUDGET: Duration = Duration::from_secs(300); /// Gap between guest readiness probes. const GUEST_READY_POLL: Duration = Duration::from_millis(25); @@ -2300,14 +2365,38 @@ impl RunnerPool

{ let building = Arc::new(AtomicUsize::new(0)); // On-demand mode: size=0 means no warm pool. Fork runners only when - // jobs arrive, capped by the host's CPU budget. + // jobs arrive, capped by the host's CPU and memory budget. if self.config.size == 0 { return self .run_on_demand(shutdown, golden_registry, idle, keys, building) .await; } - for slot in 0..self.config.size { + // Warm mode: cap the configured pool size by host memory as well as + // CPU. Each warm slot forks from the golden and inherits its + // committed footprint (growing toward `memory_mib` while a job + // runs), and a slot provisions its successor mid-job — so on a + // small host the configured size can still exhaust RAM. Sizing down + // at startup is safer than OOMing mid-run; `PRELOOP_RUNNER_POOL_SIZE` + // remains an explicit override that wins. + let warm_size = match host_memory_mib() { + Some(total) => self.config.size.min( + on_demand_memory_cap(total, u64::from(self.config.memory_mib)) + .saturating_div(2) + .max(1), + ), + None => self.config.size, + }; + if warm_size < self.config.size { + warn!( + configured = self.config.size, + warm_size, + memory_mib = self.config.memory_mib, + "reduced warm pool size to fit host memory" + ); + } + + for slot in 0..warm_size { let provider = self.provider.clone(); let config = self.config.clone(); let slot_shutdown = shutdown.child_token(); @@ -2380,7 +2469,21 @@ impl RunnerPool

{ // the fork back and spends the golden's retained checkpoint. let parallelism = std::thread::available_parallelism().map_or(2, |value| value.get()); let per_runner = usize::from(self.config.cpus.max(1)); - (parallelism / per_runner).saturating_sub(1).max(1) + let by_cpu = (parallelism / per_runner).saturating_sub(1).max(1); + // memory term: every on-demand fork inherits the golden's + // committed footprint and grows toward `memory_mib` as the guest + // runs. On a small host (the production 6-core/22 GiB machine) + // the CPU term alone allows enough 8 GiB forks to exhaust RAM + // and OOM the whole control plane. Reserve the golden's memory + // plus host headroom, then fit the remainder with per-runner + // ceilings. A host we cannot measure falls back to CPU-only. + match host_memory_mib() { + Some(total) => by_cpu.min(on_demand_memory_cap( + total, + u64::from(self.config.memory_mib), + )), + None => by_cpu, + } }; info!(max_concurrent, "on-demand runner pool (size=0)"); @@ -3617,6 +3720,31 @@ fn fork_base_unusable(error: &VmError) -> bool { .any(|signature| message.contains(signature)) } +/// Whether `golden` is a fork base this pool baked and manages. +/// +/// The plain form (`{prefix}-golden`) is the single golden baked for the +/// default environment at pool startup from the packed artifact; the +/// fingerprint-suffixed form (`{prefix}-golden-{fingerprint12}`) is prepared +/// on demand by `run_slot` from the job's OCI image when a job asks for a +/// non-default base. Only the plain form is packed — the suffixed form is an +/// environment golden whose forks do not inherit the baked baseline. Both +/// can lose their retained fork checkpoint, so the re-arm / +/// independent-create fallback applies to each; callers must not conflate +/// "managed" with "packed". +/// +/// A name like `{prefix}-golden-environment` is deliberately NOT matched: +/// that is a baked environment golden serving a different `runs-on` image, +/// and replacing it with the packed artifact would run the job on the wrong +/// operating system. +fn managed_golden(config: &RunnerPoolConfig, golden: &MachineName) -> bool { + let prefix = format!("{}-golden", config.name_prefix); + golden.as_str() == prefix + || golden.as_str().strip_prefix(&prefix).is_some_and(|rest| { + rest.strip_prefix('-') + .is_some_and(|fp| fp.len() == 12 && fp.bytes().all(|b| b.is_ascii_hexdigit())) + }) +} + /// Create, boot, and register one ephemeral runner; return its `run` argv. /// /// The caller owns cleanup: on any error the machine may already exist. @@ -3634,8 +3762,7 @@ async fn provision_runner( Some(golden) => match provider.fork(golden, name).await { Ok(()) => Some(golden), Err(error @ VmError::ForkBaseBusy { .. }) - if config.use_packed_artifact - && golden.as_str() == format!("{}-golden", config.name_prefix) => + if config.use_packed_artifact && managed_golden(config, golden) => { // A live plain-fork clone still depends on the golden's frozen // storage. Do not touch the base and do not create another VM @@ -3651,10 +3778,7 @@ async fn provision_runner( direct_create_from_packed = false; None } - Err(error) - if config.use_packed_artifact - && golden.as_str() == format!("{}-golden", config.name_prefix) => - { + Err(error) if config.use_packed_artifact && managed_golden(config, golden) => { if fork_base_unusable(&error) { // The base is spent. Re-arm it atomically with forking: // partial-clone cleanup, the live-clone check, and the @@ -3690,14 +3814,22 @@ async fn provision_runner( // A live clone (another runner forked from the // golden) blocks the re-freeze; those clones are // ephemeral and exit after their job. Wait for - // them to drain, then retry the re-arm a bounded - // number of times before falling back to direct - // creation (whose socket mount cannot serve the - // control transport, so the fallback usually - // fails registration anyway). + // them to drain, probing with exponential backoff + // so a long-running job does not force the slow + // direct-create path for every queued job in the + // meantime. The fork path is ~0.5 s vs ~8 min for + // independent creation, so waiting is worth it up + // to a generous total budget; only then fall back + // to direct creation (whose socket mount cannot + // serve the control transport, so the fallback + // usually fails registration anyway). let mut rearmed = false; - for attempt in 0..12 { - tokio::time::sleep(GOLDEN_DRAIN_PROBE_DELAY).await; + let mut probe_delay = GOLDEN_DRAIN_PROBE_DELAY; + let drain_deadline = tokio::time::Instant::now() + GOLDEN_DRAIN_BUDGET; + let mut attempt = 0_u32; + while tokio::time::Instant::now() < drain_deadline { + tokio::time::sleep(probe_delay).await; + attempt += 1; match provider.rearm_fork_base(golden, Some(name)).await { Ok(true) => { info!( @@ -3709,8 +3841,12 @@ async fn provision_runner( } Ok(false) => { // Live clones still hold the golden; - // keep probing until the bounded - // retries are exhausted. + // back off and probe again: the next + // probe costs little, and the clone + // may exit before the budget runs out. + probe_delay = probe_delay + .saturating_mul(2) + .min(GOLDEN_DRAIN_PROBE_MAX); } Err(drain_error) => { error!( @@ -3798,6 +3934,15 @@ async fn provision_runner( // so an env-golden fork boots the bare stock base image. Install the // apt baseline and toolchains into the fork itself — it is the job's // single-use machine, so the writes persist for its lifetime. + // Only the plain `{prefix}-golden` fork base is created from the + // packed artifact (`prepare_packed_golden` at pool startup), whose + // rootfs already carries the apt baseline and toolchains that forks + // inherit. Fingerprint-suffixed goldens are baked by + // `prepare_golden_for_env` from the job's OCI image via guest exec, + // and SmolVM's forkable snapshot does NOT carry post-create exec + // writes into clones — so those forks must install the baseline + // themselves. Treating an env golden as packed skipped that install + // and provisioned runners without the curated baseline. let golden_is_packed = config.use_packed_artifact && golden.as_str() == format!("{}-golden", config.name_prefix); if golden_is_packed { @@ -3858,7 +4003,19 @@ async fn provision_runner( ); } } else { - let uses_packed_artifact = direct_create_from_packed; + // The packed-artifact fallback is valid only for the plain packed + // golden: it boots the *default* OS image. A fingerprint-suffixed + // golden is an environment golden baked from the job's requested + // image, so every fork-failure fallback must boot that job's own + // environment instead — regardless of which branch failed the fork. + // When no golden was attempted (`golden` is None, the create-per- + // runner path), the packed artifact is the pool's normal image and + // stays as-is. + let golden_is_plain_packed = match golden { + Some(golden) => golden.as_str() == format!("{}-golden", config.name_prefix), + None => true, + }; + let uses_packed_artifact = direct_create_from_packed && golden_is_plain_packed; let pack = packed_golden_path(&config.artifact_payload()); let spec = MachineSpec { name: name.clone(), @@ -4260,6 +4417,35 @@ mod lifecycle_tests { use std::process::Command; use tokio::sync::Mutex; + /// The production scenario that OOMed: 22 GiB host, 8 GiB runners. + /// Golden (8 GiB) + 2 GiB reserve leaves 12 GiB → at most 1 concurrent + /// on-demand fork. CPU-only sizing allowed several and the host died. + #[test] + fn on_demand_memory_cap_fits_runners_after_golden_and_reserve() { + assert_eq!(on_demand_memory_cap(22 * 1024, 8 * 1024), 1); + assert_eq!(on_demand_memory_cap(64 * 1024, 8 * 1024), 6); + assert_eq!(on_demand_memory_cap(32 * 1024, 4 * 1024), 6); + } + + /// A host too small for even one runner past the golden still runs one + /// job (floor of 1) rather than refusing to work. + #[test] + fn on_demand_memory_cap_floors_at_one() { + assert_eq!(on_demand_memory_cap(8 * 1024, 8 * 1024), 1); + assert_eq!(on_demand_memory_cap(4 * 1024, 8 * 1024), 1); + } + + /// A zero runner ceiling (should not happen — config validates it) must + /// not divide by zero; it degrades to the 1 MiB floor and lets the host + /// run as many 1 MiB "runners" as fit. + #[test] + fn on_demand_memory_cap_handles_zero_runner_ceiling() { + assert_eq!( + on_demand_memory_cap(32 * 1024, 0), + (32 * 1024 - 2048 - 1) as usize + ); + } + #[derive(Debug)] struct TestProvider { machines: Mutex>, @@ -4273,6 +4459,9 @@ mod lifecycle_tests { /// Report live clones to `rearm_fork_base`; true by default so a spent /// base with dependents is never re-armed in tests either. live_forks: Mutex, + /// Simulate a clone exiting mid-drain: flip `live_forks` off after + /// this many `rearm_fork_base` calls (0 = never). + drain_live_forks_after: Mutex, fail_start: bool, fail_install: bool, fail_configure: bool, @@ -4305,6 +4494,7 @@ mod lifecycle_tests { fork_base_busy: false, fail_fork_once_spent: Mutex::new(false), live_forks: Mutex::new(true), + drain_live_forks_after: Mutex::new(0), fail_start, fail_install, fail_configure, @@ -4344,6 +4534,13 @@ mod lifecycle_tests { self } + /// Simulate the last live clone exiting after `n` drain probes, so a + /// re-arm that keeps probing eventually succeeds. + fn drain_live_forks_after(mut self, n: u32) -> Self { + *self.drain_live_forks_after.get_mut() = n; + self + } + /// A provider whose guests lack `binary` until an install command for /// it runs — a pack baked without the workspace's toolchain. fn without_binary(binary: &'static str) -> Self { @@ -5020,6 +5217,13 @@ chmod +x "$destination/bin/node" if let Some(partial) = partial { self.delete(partial).await?; } + let mut drain_after = self.drain_live_forks_after.lock().await; + if *drain_after > 0 { + *drain_after -= 1; + if *drain_after == 0 { + *self.live_forks.lock().await = false; + } + } if *self.live_forks.lock().await { return Ok(false); } @@ -5379,6 +5583,91 @@ chmod +x "$destination/bin/node" ); } + /// A fingerprint-suffixed golden (`{prefix}-golden-{fingerprint}`, the + /// name `run_slot` uses for a non-default base image) must be recognized + /// as managed by this pool. The stale guard matched only the plain + /// `{prefix}-golden` form, so a spent per-environment golden looped on + /// "already paused" forever instead of re-arming. + #[tokio::test] + async fn spent_fingerprint_suffixed_fork_base_is_rearmed_and_retried() { + let provider = Arc::new( + TestProvider::new(false, false, false, false, false) + .with_live_forks(false) + .failing_fork_once_spent(), + ); + let config = packed_fork_config(); + let golden = MachineName::new("lifecycle-test-golden-3577f5d5a384").unwrap(); + let name = MachineName::new("lifecycle-test-0-8").unwrap(); + + provision_runner( + &provider, + &config, + &name, + Some(&golden), + &Arc::new(KeyPool::new()), + &test_runner_environment(config.base_image.clone(), Vec::new(), true), + ) + .await + .expect("the re-armed fingerprint-suffixed golden serves the fork"); + + let events = provider.events().await; + let expected = [ + format!("fork:{}:{}", golden.as_str(), name.as_str()), + format!("rearm:{}", golden.as_str()), + format!("delete:{}", name.as_str()), + format!("stop:{}", golden.as_str()), + format!("start:{}", golden.as_str()), + format!("fork:{}:{}", golden.as_str(), name.as_str()), + ]; + let mut cursor = 0; + for event in &expected { + let position = events[cursor..] + .iter() + .position(|seen| seen == event) + .expect("re-arm sequence must include every step"); + cursor += position + 1; + } + assert!( + provider.has_machine(&name).await, + "the retried fork must leave the clone provisioned" + ); + } + + /// A fingerprint-suffixed golden is an *environment* golden baked from + /// the job's requested image. When its fork fails and the pool falls + /// back to independent creation, the runner must boot that job's + /// environment — not the default packed artifact, which would run a + /// non-default `runs-on` job on the wrong operating system. + #[tokio::test] + async fn fingerprint_golden_fork_failure_falls_back_to_the_job_environment() { + let provider = + Arc::new(TestProvider::new(false, false, false, false, false).failing_fork()); + let config = packed_fork_config(); + let golden = MachineName::new("lifecycle-test-golden-3577f5d5a384").unwrap(); + let name = MachineName::new("lifecycle-test-0-9").unwrap(); + let env = test_runner_environment("mirror.gcr.io/library/ubuntu:22.04", Vec::new(), true); + + provision_runner( + &provider, + &config, + &name, + Some(&golden), + &Arc::new(KeyPool::new()), + &env, + ) + .await + .expect("an env-golden fork failure falls back to the job environment"); + + let created = provider + .created_image(&name) + .await + .expect("the fallback created the runner machine"); + assert_eq!( + created, "mirror.gcr.io/library/ubuntu:22.04", + "an env-golden fallback must boot the job's requested image, not the default pack" + ); + } + /// A spent base that still has live clones must NOT be re-armed: resuming /// it would corrupt the copy-on-write clones. The pool falls back to a /// full create instead. @@ -5423,6 +5712,57 @@ chmod +x "$destination/bin/node" ); } + /// A live clone that exits mid-drain must be re-armed once it is gone: + /// the drain loop keeps probing with backoff, and the golden resumes + /// serving forks instead of falling back to slow direct creation. + /// Paused time advances the probe sleeps instantly. + #[tokio::test(start_paused = true)] + async fn spent_fork_base_with_clone_that_drains_is_rearmed_and_retried() { + let provider = Arc::new( + TestProvider::new(false, false, false, false, false) + .with_live_forks(true) + .drain_live_forks_after(3) + .failing_fork_once_spent(), + ); + let config = packed_fork_config(); + let golden = MachineName::new("lifecycle-test-golden").unwrap(); + let name = MachineName::new("lifecycle-test-0-10").unwrap(); + + provision_runner( + &provider, + &config, + &name, + Some(&golden), + &Arc::new(KeyPool::new()), + &test_runner_environment(config.base_image.clone(), Vec::new(), true), + ) + .await + .expect("the re-armed golden serves the fork after the clone drains"); + + let events = provider.events().await; + let expected = [ + format!("fork:{}:{}", golden.as_str(), name.as_str()), + format!("rearm:{}", golden.as_str()), + format!("rearm:{}", golden.as_str()), + format!("rearm:{}", golden.as_str()), + format!("stop:{}", golden.as_str()), + format!("start:{}", golden.as_str()), + format!("fork:{}:{}", golden.as_str(), name.as_str()), + ]; + let mut cursor = 0; + for event in &expected { + let position = events[cursor..] + .iter() + .position(|seen| seen == event) + .expect("drain-and-re-arm sequence must include every step"); + cursor += position + 1; + } + assert!( + provider.has_machine(&name).await, + "the retried fork must leave the clone provisioned" + ); + } + /// The provider reports a live clone before invoking SmolVM for another /// plain fork. The orchestrator must neither re-arm the shared golden nor /// instantiate the packed payload beside that clone. diff --git a/crates/preloop-runner-server/src/broker.rs b/crates/preloop-runner-server/src/broker.rs index 65e144cb..2dc7053e 100644 --- a/crates/preloop-runner-server/src/broker.rs +++ b/crates/preloop-runner-server/src/broker.rs @@ -778,10 +778,162 @@ pub(crate) async fn broker_acquire_job( let mut inner = shared.state.inner.lock().await; inner.broker_messages.insert(request_id, message.clone()); } else { - tracing::warn!( - request_id, - "broker acquire: no dispatch token request for job" - ); + // A token request registered at build time can be lost when the + // process dies before the next store snapshot flush (jobs enqueued + // since the last snapshot restore with `github_token_requests` + // missing). The claim then reaches here with no request to mint + // from, the checkout keeps the local runtime JWT, and every git + // fetch fails on auth. Re-derive the request from the run's + // submission and the job's declared permissions — the same inputs + // `build_job_artifacts` used — and mint under that policy. + let derived = if shared.state.github_app.is_none() { + None + } else { + let inner = shared.state.inner.lock().await; + let record = inner.job_requests.get(&request_id); + let run = record.and_then(|record| inner.runs.get(&record.run_id)); + match (record, run) { + (Some(record), Some(run)) => { + // The submission stores the tier as a plain kebab-case + // string (e.g. "untrusted-fork-pull-request"), not JSON. + // `from_str` expects JSON and would reject the bare + // string, yielding `None` — which `job_authorization` + // treats as trusted, silently un-restricting a fork + // job's token. Parse via a JSON string value so the + // kebab-case variant decodes. + let tier = run.submission.trust_tier.as_deref().and_then(|tier| { + serde_json::from_value(serde_json::Value::String(tier.to_owned())).ok() + }); + // The job's resolved permission set lives in the + // persisted message's `system.github.token.permissions` + // variable (PascalCase wire spelling) — the same + // variable the build path wrote from `JobPlan` + // permissions. The event payload's `workflow_job` key is + // absent for push/PR/dispatch events, so reading it there + // would fall back to the broad default and grant scopes + // the workflow withheld. Recover from the message + // instead, converting the wire spelling back to + // kebab-case for the token request. + let wire_permissions = message + .variables + .get("system.github.token.permissions") + .and_then(|variable| variable.value.as_deref()) + .and_then(|json| { + serde_json::from_str::>(json).ok() + }); + // The wire variable spells scopes PascalCase + // ("PullRequests"); the installation-token request and + // `job_authorization` expect the workflow's kebab-case + // identities ("pull-requests"). Minting with PascalCase + // keys fails (or falls back to the broad PAT), so + // convert every key before building the request. + let wire_permissions = wire_permissions.map(|permissions| { + permissions + .into_iter() + .map(|(scope, level)| (wire_scope_to_kebab(&scope), level)) + .collect::>() + }); + // `system.github.token.permissions` carries the effective + // set (defaults substituted when nothing was declared). + // Passing it as the declared set is faithful: for a + // declared job it is exactly the job's set, and for an + // undeclared job `job_authorization` treats a set equal + // to the default identically to `None`. The fork case is + // safe too — the wire variable was restated to the fork + // profile at build, and clamping it again is idempotent. + let declared = wire_permissions.clone(); + let policy = crate::events::trust_tier::job_authorization( + tier, + declared.as_ref(), + false, + ); + Some(( + crate::models::GitHubTokenRequest { + repository: run.submission.repository.clone(), + permissions: policy.app_permissions, + declared: declared.is_some(), + untrusted: policy.fork_restricted, + }, + record.request_id, + )) + } + _ => None, + } + }; + if let Some((token_request, derived_request_id)) = derived { + // Register the derived request so a re-claim after a disconnect + // re-mints under the same derived policy, then mint. + { + let mut inner = shared.state.inner.lock().await; + inner + .github_token_requests + .insert(derived_request_id, token_request.clone()); + } + tracing::info!( + request_id, + repository = %token_request.repository, + "broker acquire: re-derived missing dispatch token request at claim" + ); + let minted = match mint_dispatch_github_token(&shared, &token_request).await { + Ok(minted) => minted, + Err(error) => { + fail_unclaimable_request(&shared, request_id).await; + return Err(error); + } + }; + if let Some(minted) = minted { + let token = minted.token; + tracing::info!( + token_len = token.len(), + "minted re-derived dispatch GitHub token at claim" + ); + message.variables.insert( + "system.github.token".to_owned(), + preloop_gha_protocol::azdo::VariableValue::secret(token.clone()), + ); + message.variables.insert( + "github_token".to_owned(), + preloop_gha_protocol::azdo::VariableValue::secret(token.clone()), + ); + message.variables.insert( + "GITHUB_TOKEN".to_owned(), + preloop_gha_protocol::azdo::VariableValue::secret(token.clone()), + ); + // Restate what the token carries when the installation could + // not grant everything, mirroring the normal mint path: the + // recovered message's wire set is the requested set, and + // leaving it would print authority the token does not have. + if let Some(effective) = minted.effective_permissions { + let merged = merge_narrowed_wire_permissions( + message + .variables + .get("system.github.token.permissions") + .and_then(|variable| variable.value.as_deref()), + &effective, + ); + message.variables.insert( + "system.github.token.permissions".to_owned(), + preloop_gha_protocol::azdo::VariableValue::new( + preloop_gha_parser::job_builder::token_permissions_wire_json(&merged), + ), + ); + } + match message.context_data.get_mut("github") { + Some(preloop_gha_protocol::azdo::PipelineContextData::Dict(github)) => { + github.insert( + "token".to_owned(), + preloop_gha_protocol::azdo::PipelineContextData::String(token), + ); + } + other => tracing::warn!( + github_context = %match other { Some(_) => "non-dict", None => "missing" }, + "could not patch github context token for re-derived request" + ), + } + let mut inner = shared.state.inner.lock().await; + inner.broker_messages.insert(request_id, message.clone()); + } + } } // The snapshot checkout token is pinned onto the step at submission, // but a job can sit queued well past its ~50-minute lifetime. The diff --git a/crates/preloop-runner-server/src/results_twirp.rs b/crates/preloop-runner-server/src/results_twirp.rs index dbcfd84c..7938b99c 100644 --- a/crates/preloop-runner-server/src/results_twirp.rs +++ b/crates/preloop-runner-server/src/results_twirp.rs @@ -996,8 +996,14 @@ mod cache_pb_tests { metadata: Some(PbCacheMetadata { repository_id: 42, scope: vec![ - PbCacheScope { scope: "refs/heads/main".to_string(), permission: 1 }, - PbCacheScope { scope: "refs/heads/feature".to_string(), permission: 2 }, + PbCacheScope { + scope: "refs/heads/main".to_string(), + permission: 1, + }, + PbCacheScope { + scope: "refs/heads/feature".to_string(), + permission: 2, + }, ], }), key: "k".to_string(), @@ -1015,7 +1021,10 @@ mod cache_pb_tests { let fixture = include_bytes!("../../../fixtures/wire/cache-multi-scope.pb"); let (_, _, _, fixture_scopes, _) = pb_cache_request(fixture, CacheRequestKind::GetDownloadUrl).unwrap(); - assert_eq!(fixture_scopes, vec!["refs/heads/main", "refs/heads/feature"]); + assert_eq!( + fixture_scopes, + vec!["refs/heads/main", "refs/heads/feature"] + ); } #[test] diff --git a/crates/preloop-runner-server/src/store.rs b/crates/preloop-runner-server/src/store.rs index 56c86110..0509e7f6 100644 --- a/crates/preloop-runner-server/src/store.rs +++ b/crates/preloop-runner-server/src/store.rs @@ -18,8 +18,8 @@ use async_trait::async_trait; use preloop_gha_protocol::SessionId; use rusqlite::{params, Connection, OptionalExtension, Transaction}; use sha2::Digest; -use std::sync::Mutex as StdMutex; use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::Mutex as StdMutex; const DATABASE_FILE: &str = "preloop.db"; pub(crate) const SNAPSHOT_FORMAT: u8 = 2; @@ -555,9 +555,18 @@ pub(crate) fn restore_run_record(cipher: &Envelope, blob: &[u8]) -> anyhow::Resu .unwrap_or_default() .to_owned(); // `run_record_value` always writes the key (null when absent), so a - // JSON null must restore as `None` rather than fail to parse. + // JSON null must restore as `None` rather than fail to parse. A + // snapshot whose shape this binary no longer understands is dropped + // with a warning: the store is best-effort and one stale record must + // not brick startup (see `load_into`). run.workspace_snapshot = match object.get("workspace_snapshot") { - Some(value) if !value.is_null() => Some(serde_json::from_value(value.clone())?), + Some(value) if !value.is_null() => match serde_json::from_value(value.clone()) { + Ok(snapshot) => Some(snapshot), + Err(error) => { + tracing::warn!(run_id = %run.run_id, %error, "dropping undecodable workspace snapshot on load"); + None + } + }, _ => None, }; } diff --git a/docs/retries-and-pool-memory.md b/docs/retries-and-pool-memory.md new file mode 100644 index 00000000..d81d8c13 --- /dev/null +++ b/docs/retries-and-pool-memory.md @@ -0,0 +1,138 @@ +# Retry, backoff, and pool memory bounds + +How preloop retries failures, when it backs off, and how the runner pool +keeps itself inside the host's memory budget. Written after the 2026-08-20 +outage, where a 22 GiB production host OOMed because on-demand fork +concurrency was sized by CPU only. + +## Retry philosophy, in one line + +> **Server: retry-once-and-coordinate** — the control plane avoids in-process +> retry loops almost entirely, leaning on GitHub redelivery, runner +> long-polls (bounded by `waitSeconds`), and periodic reaper sweeps. +> **Runner: mirror the official runner** — bounded 3-attempt exponential HTTP +> retries, jittered session backoff (15–60 s) reset on success, bounded lease +> renewal; listener/session loops and the container-health poll retry forever +> by design (the guest is expected to recover). +> **Pool: retry-forever with exponential damping** — slot respawns back off +> 500 ms → 30 s cap with success reset, golden re-arm exponential (10 s → 60 s, +> ≤ 5 min budget) before fallback +> before fallback. + +## Control plane (`preloop-runner-server`) + +| Site | What | Policy | Bounded? | +|---|---|---|---| +| `github.rs` `resolve_check_run_token` | App check-run token mint, transient 422 | 2 attempts, 500 ms fixed | yes | +| `github_app.rs` `mint_for_repository` | Installation token mint, 422 ungranted scope | one re-mint with clamped permissions | yes | +| `store.rs` `checkpoint_wal` | SQLite `wal_checkpoint(TRUNCATE)` blocked | 10 × 10 ms (blocking sleep), then bail | yes (~100 ms) | +| `debug.rs` `pump_axum_ws_to_dap` | DAP bridge WebSocket connect | 50 × 200 ms (≤10 s) | yes | +| `snapshots.rs` `acquire_cache_lock` | Object-cache dir lock | 25 ms poll, stale >60 s force-removed, 10 s deadline | yes | +| `broker.rs` / `distributed_task.rs` | Job-claim long poll | `loop` until `wait` deadline (default 50 s), wakes on notify ≤3 s slices | yes | +| `debug_sessions.rs` `poll_verdict` / `agent_events` | Worker verdict / events long poll | `loop` until 25 s cap (`VERDICT_POLL_MAX`) | yes | +| `bootstrap.rs` `run_background_reaper` | Stalled job/session sweep | 10 s interval, process lifetime | wall-time unbounded, periodic | +| `actions.rs` / `state.rs` | Action ref → SHA resolution | negative cached 60 s, positive 300 s; re-attempt after TTL | TTL-bounded | +| `broker.rs` `broker_acquire_job` | Dispatch token mint refusal (error policy) | **deliberately no retry** — config fault, job failed terminally | n/a | + +GitHub webhooks are **not retried in-process**: the reservation is released +on failure so GitHub's own redelivery is accepted (dedup window 300 s). + +## Pool (`preloop-orchestrator`) + +| Site | What | Policy | Bounded? | +|---|---|---|---| +| `run_on_demand` slot supervisor | Re-spawn failed slots | exp 500 ms → 30 s cap, reset to 0 on success | attempts unbounded, sleep ≤30 s | +| `run_on_demand_slot` provision failure | Fork/create failed | fixed 500 ms + continue; no counter | **unbounded** (tight-ish) | +| `provision_runner` golden re-arm | Spent checkpoint after clone drain | exponential: 10 s → 60 s cap, total 300 s budget, then direct OCI create | yes (≤5 min) | +| `await_guest_ready` | Guest agent readiness probe | 25 ms poll, 30 s deadline | yes | +| golden download | Release/OCI asset | single attempt, 1 h timeout, then local-build fallback | n/a | +| `preload_images` / `docker_start_command` | dockerd readiness / start | shell polls (30 s / 10 s+5 s), start retried once | yes | + +## SmolVM provider (`preloop-vm`) + +| Site | What | Policy | Bounded? | +|---|---|---|---| +| `delete` | `smolvm machine delete` | 3 attempts: "directory not empty" 100 ms, "database is locked" 500 ms; "not found" = success | yes | + +fork / create / exec / start / stop have **no retry** — errors propagate to +the orchestrator, which retries at the slot level. + +## Runner (`preloop-runner`) + +| Site | What | Policy | Bounded? | +|---|---|---|---| +| `client/http.rs` POST/PUT | AzDO/broker JSON, log append | 3 attempts, exp 2 s / 4 s, transient (5xx / network) only | yes | +| `client/http.rs` `SessionBackoff` | Listener reconnect | jittered: ≤5 errors [15,30) s, then [30,60) s; reset on success | unbounded attempts, long sleeps | +| `broker_listener.rs` `run_broker_loop` | Session create + message poll | SessionBackoff; 409 retriable; OAuth linear `min(n*5,60)` s; deprecated → exit | unbounded (until shutdown) | +| `message_listener.rs` | Session create + poll (classic) | conflict: 8 × 30 s (~4 min); transient: 30 s fixed, no cap | conflict yes, transient **unbounded** | +| `container_ops.rs` `wait_for_services_healthy` | Docker service health poll | exp 2 → 32 s (official GetExponentialBackoff) | **unbounded attempts** | +| `container_ops.rs` `docker_registry_login` | `docker login` | 3 attempts, 5 s / 10 s | yes | +| `debug_pause.rs` `await_verdict` | Debug verdict poll | fixed 2 s; **infinite by design** (server suspends timeout) | unbounded by design | +| `live_logs.rs` send / connect | Live-log WebSocket | 3 attempts, random 100–500 ms backoff, 30 s connect timeout | yes | +| `steps_runner.rs` `run_steps` | Debug `:retry` verdicts | capped at `MAX_DEBUG_ATTEMPTS` = 25, then fail | yes | +| `job_runner.rs` `first_renew_gate` | First `renewjob` | 5 retries, random 1–10 s; 404 → Abandoned | yes | +| `job_runner.rs` renew loop | Lease renewal (every 60 s) | random 5–15 s (first 5), 15–30 s after; lease expired `LockedUntil + 5 min`; 401 → re-acquire once; 404 → cancel | yes (lease window) | +| `reporting.rs` `flush_step_updates` | Timeline publish | failed → requeue, retried every 500 ms drain tick | **unbounded attempts** | +| `control_bridge.rs` | TCP splice to upstream | bridge never exits; runner's poll loop retries forever through it | architecture-level | + +## Runner client (`preloop-runner-client`) + +No retry loops — single-shot submits with a reqwest timeout. + +## Gaps worth knowing + +1. **`run_on_demand_slot` provision failure retries at fixed 500 ms, unbounded.** + The slot-supervisor exponential backoff above it does **not** damp this + inner loop — a broken smolvm spins ~2 attempts/s. Not the OOM cause, but + noisy; a per-slot attempt counter with escalation would be an improvement. +2. **`wait_for_services_healthy` polls forever** (exp 2→32 s, no cap). Only + the outer job timeout ends it. Matches the official runner, so changing + it needs a fidelity note. +3. **`flush_step_updates` retries every 500 ms forever** on a permanently + failing publish endpoint. Bounded CPU, but unbounded wall time. +4. **`reporting.rs` requeue** is retry-by-drain-tick; harmless but + unbounded. + +## Pool memory bounds (the OOM guard) + +Before 2026-08-20, on-demand fork concurrency was sized **by CPU only**: + +``` +(available_parallelism / cpus_per_runner) - 1 // floor 1 +``` + +On the 6-core / 22 GiB production host with `PRELOOP_RUNNER_MEMORY_MIB=8192`, +that allowed enough 8 GiB forks to exhaust RAM and OOM the control plane — +the golden alone commits 8 GiB, every fork inherits that footprint and grows +toward the ceiling while its job runs, and warm mode provisions a successor +mid-job (so `size + 1` VMs are live). + +The guard, `on_demand_memory_cap`: + +``` +runner_mib = memory_mib.max(1) +golden_mib = runner_mib +by_memory = (host_total - golden_mib - 2048 MiB reserve) / runner_mib // floor 1 +max_concurrent = min(cpu_term, by_memory) +``` + +- The 2 GiB reserve keeps the control plane, OS, and page cache alive — + without it the host OOMs *after* the forks are up. +- Applied in **both** pool modes: + - size=0 on-demand: `max_concurrent = min(by_cpu, by_memory)` + - warm mode: `warm_size = min(configured size, by_memory)`, logged when + reduced. `PRELOOP_RUNNER_POOL_SIZE` still wins as an explicit override + only up to the memory cap — the cap is a safety floor, not a knob. +- A host whose memory can't be read (`/proc/meminfo` unavailable, non-Unix) + falls back to CPU-only sizing rather than refusing to run. +- Floors at 1 so a tiny host still runs a single job. + +### Test vectors (`on_demand_memory_cap`) + +| Host | Runner ceiling | Cap | +|---|---|---| +| 22 GiB | 8 GiB | 1 (production case) | +| 64 GiB | 8 GiB | 6 | +| 32 GiB | 4 GiB | 6 | +| 8 GiB | 8 GiB | 1 (floor) | +| 4 GiB | 8 GiB | 1 (floor) |