Skip to main content

max / makenotwork

ops-exec: bound the agent control calls, leave builds unbounded AgentRpc built a default reqwest client, which has no timeouts at all. An unreachable agent only ever failed because the OS gave up on the connect (~30s) -- luck, not design -- and an agent that accepts TCP without ever answering (a wedged process, or a path that blackholes after the handshake) hung a release forever with no output, under a main.rs comment promising "Fail fast if the agent isn't reachable / in-session". Set connect_timeout(5s) on the client: it bounds establishing the connection only, never a live response body, so /run and /pull keep their unbounded streams. Cap /health per-request at 10s, and say which of the two failures it was. NOT a client-level timeout: reqwest's request timeout runs until the response body has finished, so it would sever a real build mid-flight and read as a build failure. agent_e2e pins that -- a step sleeping past the health cap must survive; it fails at exactly 10s if a client timeout is ever added. Verified live through the driver: wedged agent 10.0s (was: forever) with "no answer within 10s -- ... or it is accepting connections without answering them"; unreachable 5.0s (was 30.3s); 12s build unsevered.
Author: Max Johnson <me@maxj.phd> · 2026-07-17 00:25 UTC
Commit: 91f9feafa712bcd8f0981e0c10da32284bb73955
Parent: af013d2
2 files changed, +66 insertions, -7 deletions
M shared/ops-exec/src/rpc.rs +39 -7
@@ -18,6 +18,7 @@ use anyhow::{Context, Result};
18 18 use async_trait::async_trait;
19 19 use futures_util::StreamExt;
20 20 use std::path::Path;
21 + use std::time::Duration;
21 22
22 23 /// A handle to one `ops-agent`, scoped to a caller-side capability set.
23 24 pub struct AgentRpc {
@@ -27,16 +28,39 @@ pub struct AgentRpc {
27 28 client: reqwest::Client,
28 29 }
29 30
31 + /// How long to wait for the TCP connect before calling an agent unreachable.
32 + ///
33 + /// Applies to every request, including `/run` and `/pull`: it bounds only
34 + /// establishing the connection, never a live response body, so a multi-minute
35 + /// build or a large artifact download is unaffected. The agent is one tailnet
36 + /// hop away, so a connect that has not landed in this long is not going to.
37 + const CONNECT_TIMEOUT: Duration = Duration::from_secs(5);
38 +
39 + /// Whole-request cap for the short control calls (`/health`), which return a
40 + /// small JSON body immediately.
41 + ///
42 + /// Deliberately NOT applied to the client as a whole: `reqwest`'s request
43 + /// timeout runs until the response *body* has finished, so a client-level
44 + /// timeout would sever `/run` mid-build and truncate `/pull` on a large DMG.
45 + /// Those two are unbounded by design — a build legitimately runs for minutes —
46 + /// and their liveness is covered by `CONNECT_TIMEOUT` plus the exit frame that
47 + /// `run_streaming` requires before it will report success.
48 + const HEALTH_TIMEOUT: Duration = Duration::from_secs(10);
49 +
30 50 impl AgentRpc {
31 51 /// `base_url` is e.g. `http://mbp.tailnet:8765` (the agent listens on the
32 52 /// tailnet interface). `host_label` is used only for audit messages.
33 53 pub fn new(base_url: impl Into<String>, host_label: impl Into<String>, caps: CapabilitySet) -> Self {
34 - Self {
35 - base_url: base_url.into(),
36 - host_label: host_label.into(),
37 - caps,
38 - client: reqwest::Client::new(),
39 - }
54 + // A default-built client has no timeouts whatsoever, which left an agent
55 + // that accepts TCP and then never answers (a wedged process, or a path
56 + // that blackholes after the handshake) hanging a release indefinitely
57 + // with no output. An unreachable host only ever failed because the OS
58 + // gave up on the connect (~30s), which was luck, not design.
59 + let client = reqwest::Client::builder()
60 + .connect_timeout(CONNECT_TIMEOUT)
61 + .build()
62 + .expect("reqwest client with a connect timeout");
63 + Self { base_url: base_url.into(), host_label: host_label.into(), caps, client }
40 64 }
41 65
42 66 /// `GET /health` — liveness plus the agent's own declared grant.
@@ -44,9 +68,17 @@ impl AgentRpc {
44 68 let resp = self
45 69 .client
46 70 .get(format!("{}/health", self.base_url))
71 + .timeout(HEALTH_TIMEOUT)
47 72 .send()
48 73 .await
49 - .context("GET /health")?
74 + .with_context(|| {
75 + format!(
76 + "GET /health ({}): no answer within {}s — the agent is unreachable, or it is \
77 + accepting connections without answering them",
78 + self.host_label,
79 + HEALTH_TIMEOUT.as_secs()
80 + )
81 + })?
50 82 .error_for_status()
51 83 .context("agent /health status")?;
52 84 resp.json().await.context("decoding /health")
M shared/ops-exec/tests/agent_e2e.rs +27
@@ -25,6 +25,33 @@ async fn spawn_agent(allow: Vec<CallerGrant>, grant: GrantConfig) -> String {
25 25 spawn_agent_with_pull(allow, grant, None).await
26 26 }
27 27
28 + /// A build legitimately runs for minutes, so `/run` must have no whole-request
29 + /// timeout — only `/health` does. This is the trap in the timeout work
30 + /// (2026-07-16): `reqwest`'s request timeout runs until the response *body* has
31 + /// finished, so a client-level timeout would sever a real build mid-flight and
32 + /// look exactly like a build failure. Sleeps past `HEALTH_TIMEOUT` (10s) to
33 + /// prove the cap is not applied here. Slow on purpose — it is the only thing
34 + /// standing between a future "just set a client timeout" and a severed release.
35 + #[tokio::test]
36 + async fn a_step_outliving_the_health_timeout_is_not_severed() {
37 + let base = spawn_agent(
38 + vec![CallerGrant {
39 + identity: "fw13".into(),
40 + actuate: vec!["build".into()],
41 + observe: vec![],
42 + }],
43 + builder_grant(),
44 + )
45 + .await;
46 + let rpc = AgentRpc::new(base, "mbp", CapabilitySet::from_tokens(["build"], Vec::<&str>::new()));
47 +
48 + let mut sink = VecSink::default();
49 + let step = Step::shell(Action::Build, "sleep 12; printf 'built'");
50 + let out = rpc.run_streaming(&step, &mut sink).await.expect("a 12s build must not be cut off");
51 + assert!(out.success());
52 + assert_eq!(out.stdout, b"built");
53 + }
54 +
28 55 /// As [`spawn_agent`], but with a configurable `pull_root` for the file-read path.
29 56 async fn spawn_agent_with_pull(
30 57 allow: Vec<CallerGrant>,