fix(orchestrator): bound host externals, orphan sweeps, and golden metadata waits so a hung dependency cannot freeze the engine

Merged
BN
2d ago
+1067-181

Stacks on #685; retarget to main after it merges.

A hung external dependency must end in an error, not a freeze. Commit 44d51b523 bounded the OS credential store and moved TLS verification in-process; this closes the remaining unbounded waits in preloop-orchestrator from the blocking-call audit (rows #3, #6 orchestrator side, and G10). Every wait below now runs through preloop_observability::deadline (from #685), so nothing here hand-rolls a timeout, a kill, or a thread guard.

What was unbounded

GapSiteWaited onNow
#3crates/preloop-orchestrator/src/lib.rs:165-196 ensure_host_externals, called from async fn run at :4266synchronous sh -c … .status() for the install script built by node_externals_at (:1966-2034), whose curl -fsSL had no --connect-timeout/--max-timedeadline::run_command_status, HOST_EXTERNALS_INSTALL_TIMEOUT (30 min); curl carries --connect-timeout 30 --max-time 600 per archive and --max-time 120 for SHASUMS256.txt
#3 (same flow)bundle and mirror cp -a in ensure_host_externals and materialize_mirror_bundlesynchronous cp … .output() on the async workerdeadline::run_command_status, HOST_EXTERNALS_COPY_TIMEOUT (5 min)
#6 (orchestrator)lib.rs:1206-1258 periodic reconciler and lib.rs:4952-4978 startup sweeppreloop_vm::purge_orphaned_vms / detach_orphaned_layer_mounts (ps ax, kill -9, /sbin/mount, hdiutil detach) synchronously on a tokio workerdeadline::run_blocking, HOST_CLEANUP_TIMEOUT (10 min)
G10resolve_latest_golden_tag (:619-668), OCI manifest/token fetch, .sha256 sidecarthe 60-minute GOLDEN_DOWNLOAD_TIMEOUT client (:1152)GOLDEN_METADATA_TIMEOUT (30 s) via one golden_http_client(timeout) helper; downloads keep the hour
—host_memory_mib (sysctl -n hw.memsize)unbounded child on the async workerdeadline::run_command, HOST_PROBE_TIMEOUT (10 s), non-fatal (falls back to CPU-only sizing)
—is_valid_externals_dir in node_externals.rsunbounded bin/node --version child on the validation pathdeadline::run_command_blocking, NODE_VERSION_PROBE_TIMEOUT (10 s); a hang counts the directory invalid, so the next start re-materializes it

Why these deadlines

  • 30 min for the install: the script moves ~104 MB (57 MB + 47 MB archives plus a 3 KB checksum file, measured from nodejs.org), so the budget is a floor on link speed like GOLDEN_DOWNLOAD_TIMEOUT, not a formality: ~0.5 Mbps sustained fits. A healthy host finishes in well under a minute. The per-transfer curl limits sum to 24 min, so a stalled transfer ends with curl's own error and only a hang the limits cannot see reaches the outer deadline.
  • 5 min for a local cp -a of a few hundred MB: covers a cold page cache or a release directory behind a network mount.
  • 10 min for orphan cleanup: preloop-vm bounds each of its children at 30 s (their own change), so this covers roughly eighteen children that each hit their own bound; past that the caller stops waiting and the sweep finishes on its own.
  • 30 s for metadata: a release listing, an OCI manifest, a bearer token, and a .sha256 sidecar are small bodies, and each lookup is best effort with a fallback, so giving up early costs a fallback and never a failed start.
  • 10 s for sysctl and node --version: both answer in milliseconds; the deadline only has to survive a loaded host.

Error messages name the dependency and the deadline (CommandError/BlockingError display "{what} timed out after {deadline:?}"), so an operator can tell a hang from a failure.

Tests

  • host_externals_bound_tests::a_hung_host_externals_step_is_killed_at_its_deadline — a fake step that records its pid and sleeps forever; the deadline returns an error naming the step, and the recorded pid is gone afterwards (proves the kill, not just the return).
  • host_externals_bound_tests::a_stalled_node_download_ends_at_curl_max_time — the real node_externals_script against a listener that accepts and never answers: ends at curl's own limit, publishes no runtime.
  • golden_download_tests::stalled_golden_release_lookup_gives_up_at_the_metadata_deadline — a stalled local listener makes the release lookup fail at the metadata deadline. Fail-before evidence: run with golden_http_client(GOLDEN_DOWNLOAD_TIMEOUT) it waits 3600 s and the test fails with waited 3600s: the lookup ran on the download budget.
  • node_externals::tests::a_node_binary_that_never_answers_is_killed_at_the_probe_deadline / …that_answers_reports_its_version — the probe waits out its deadline and kills the child; a healthy interpreter still validates.

Two test-fixture races this exposed

Bounding these waits added real await points to the pool's startup path, which surfaced two orderings the integration fixtures relied on. Both were fixed in tests/runner_pool_lifecycle.rs, not by weakening the production change:

  1. artifact_preparation_runs_once_and_reuses_payload_on_next_run cancelled the first pool run the instant the pack landed and then waited for a cumulative second run call. Whether the cancelled pass boots a runner before its slot observes the shutdown depends on where the pool yields between the pack and the slot's first shutdown check. It now waits for one run call more than the first pass produced (ProviderStateSnapshot gained run_calls).
  2. The paused-clock tests (preserved_runner_expires_at_idle_timeout_without_heartbeat, removing_debug_marker_releases_preserved_runner_on_next_poll) drove a real cp through tokio::time::pause(): a paused runtime advances the clock to the child's deadline the moment it goes idle, so the copy was killed and pool start failed. The fixture now stages the bundle's own externals as well as the host copy, which is the release layout the pool mounts without copying; the copy path stays covered by the non-paused lifecycle tests. Without this, both tests hung for the full 50-minute cap instead of passing in seconds.

Gates

  • cargo fmt --all --check — clean.
  • cargo clippy --locked -p preloop-orchestrator --all-targets -- -D warnings — clean.
  • cargo test -p preloop-orchestrator --locked — 155 lib tests passed, 14 runner_pool_lifecycle integration tests passed (32 s), 0 doctests.
  • Verified against the baseline commit 44d51b523 in a scratch worktree while diagnosing the two races above; no other crate's tests were run.

Deliberately left

  • G10 does not go through the GitHub breaker. send_observed needs a GithubBreakerRegistry, which lives in AppState (preloop-runner-server/src/state.rs:894) and is not reachable from the orchestrator: RunnerPoolConfig has no handle for it, and the CLI builds the pool beside the server task whose bootstrap::serve constructs AppState. Routing the anonymous release lookup (GithubCredentialBucket::Anonymous) through the breaker needs a registry handle plumbed from serve into RunnerPoolConfig — a CLI/bootstrap change outside this slice. Reported to Main and to the breaker slice rather than added here.
  • The OCI blob request keeps the long client, including the bearer-token exchange inside registry_get on a 401. Splitting that needs oci.rs's signature to change, and oci.rs is shared with preloop-cli's manifest inspector. The manifest fetch itself is on the metadata client.
  • Killing the install child does not reach its grandchildren. kill_on_drop kills the sh -c script, not the curl it started; each curl carries its own --max-time so none outlives its transfer budget. Killing the process group would need a pre-exec hook.
  • is_valid_externals_dir stays synchronous (with a deadline) because preloop-runner configure also calls it from a non-async context.

Devin Review


Summary by cubic

A hung external dependency now ends in a named error instead of freezing pool start: every unbounded host subprocess and golden metadata wait in preloop-orchestrator goes through preloop_observability::deadline, which kills and reaps children at the deadline.

What is now bounded

  • The host node externals install runs with a 30-min deadline and per-transfer curl limits (--connect-timeout 30 --max-time 600, 2 min for the checksum file); the bundle and mirror cp -a copies get 5 min. The install runs in its own process group, so expiry or a dropped future kills its curl/shasum/tar grandchildren, and RunnerPool::run awaits it under unless_shutdown so a stop cancels it instead of waiting out the 30 minutes.
  • The node --version externals probe gets 10 s, with a hang counted as invalid so the next start re-materializes the directory. A non-native binary that starts and hangs is a wedged runtime (TimedOut), not a foreign binary the host cannot execute, and only NotRunnable may fall through to the non-empty-file check. The sysctl memory probe gets 10 s and falls back to CPU-only sizing on failure.
  • Orphaned hypervisor purge and layer-mount detach (startup and periodic reconcile) stop waiting after 10 min, and preloop-vm now bounds each child at 30 s so the caller's deadline is a backstop. A startup OrphanPurge::All sweep that outlives the caller deadline fails pool start; teardown keeps the warning.
  • Golden release listing, OCI manifest, and .sha256 lookups use a 30 s client; downloads keep the 60-min budget. The blob's bearer-token exchange moved to the metadata client; oci.rs takes both clients, and the CLI's manifest inspector passes its own.

Tests and fixture fixes

  • New tests prove a hung install step is killed at its deadline, its sleeping grandchild dies with the process group at expiry and on a dropped future, a stalled node download ends at curl's own limit, a stalled metadata lookup gives up at 30 s, a wedged node probe is killed, and a hung foreign-platform runtime fails validation.
  • Two integration-fixture races surfaced: the reuse test waits for one run call beyond the first cancelled pass, and paused-clock tests stage the bundle's own externals so no real cp runs under a paused runtime. The healthy externals fixture reuses the exec warm-up helper so concurrent test forks cannot trip the bounded probe.