From 4ed256aac3a0b540e754770c6a6671d9ee51ec37 Mon Sep 17 00:00:00 2001 From: Robert DeLanghe <1240090+bdelanghe@users.noreply.github.com> Date: Fri, 28 Aug 2026 14:04:19 -0400 Subject: [PATCH 1/2] feat(share): make the upload timeout settable, and raise the default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One flat 30s covered every pathbase call. That is a fine ceiling for the small ones — auth, listing repos, deleting a graph — and the wrong one for an upload: a long agent session derives to several MB and the whole document goes up in a single request. Hit for real against a LOOPBACK server, which is the case that should never time out. The message it produces is the tell: request timed out after 30s — try again, or shrink the upload You cannot shrink a conversation that already happened, so the advice has nowhere to go, and a session gets quietly dropped rather than stored — in a tool whose whole job is not dropping sessions. The autoload sweep hit the same wall every ten minutes with the same result. Default is now 300s, and `--timeout ` on `share` overrides it. `PATH_HTTP_TIMEOUT_SECS` does the same for every command, including the ones with no flag of their own — which is what a launchd agent or a cron sweep can reach. Set via the environment rather than threaded as a Duration: the eight `pathbase_client` call sites are all small requests that never needed a knob, and giving them one to pass along would be noise at each of them. `run` sets it before any client is constructed. Note this raises a ceiling; it does not make an upload fast. A 4 MB document taking more than 30s against loopback is its own question — the API is a debug build under the hot-reload service, which is the first thing to check. --- crates/path-cli/src/cmd_pathbase.rs | 26 +++++++++++++++++++++++++- crates/path-cli/src/cmd_share.rs | 21 +++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/crates/path-cli/src/cmd_pathbase.rs b/crates/path-cli/src/cmd_pathbase.rs index bfff3f7b..c824875b 100644 --- a/crates/path-cli/src/cmd_pathbase.rs +++ b/crates/path-cli/src/cmd_pathbase.rs @@ -349,10 +349,34 @@ fn block_on(f: F) -> F::Output { /// supplied. Progenitor doesn't expose a bearer-token setter, so we /// pre-bake the header into the http client and hand it via /// `Client::new_with_client`. +/// How long any pathbase request may take. +/// +/// One flat value covered every call, and 30s is a fine ceiling for the +/// small ones — auth, listing repos, deleting a graph. It is the wrong +/// ceiling for an upload: a long agent session derives to several MB, and +/// the whole document goes up in one request. A real session hit this +/// against a *loopback* server, which is the case that should never time +/// out, and the error it produced — "request timed out after 30s — try +/// again, or shrink the upload" — invites you to shrink a conversation +/// that already happened. +/// +/// So the default is generous and the value is settable. `PATH_HTTP_TIMEOUT_SECS` +/// covers every command, including the ones with no flag of their own — which +/// is what a launchd agent or a cron sweep can reach. +pub(crate) fn http_timeout() -> std::time::Duration { + const DEFAULT_SECS: u64 = 300; + let secs = std::env::var("PATH_HTTP_TIMEOUT_SECS") + .ok() + .and_then(|s| s.trim().parse::().ok()) + .filter(|s| *s > 0) + .unwrap_or(DEFAULT_SECS); + std::time::Duration::from_secs(secs) +} + fn pathbase_client(base_url: &str, token: Option<&str>) -> Result { let mut builder = reqwest::Client::builder() .user_agent(concat!("path-cli/", env!("CARGO_PKG_VERSION"))) - .timeout(std::time::Duration::from_secs(30)); + .timeout(http_timeout()); if let Some(t) = token { let mut headers = reqwest::header::HeaderMap::new(); let mut auth = reqwest::header::HeaderValue::from_str(&format!("Bearer {t}")) diff --git a/crates/path-cli/src/cmd_share.rs b/crates/path-cli/src/cmd_share.rs index 0d6ba9c4..9a762bfe 100644 --- a/crates/path-cli/src/cmd_share.rs +++ b/crates/path-cli/src/cmd_share.rs @@ -57,6 +57,15 @@ pub struct ShareArgs { /// Skip writing the cache; derive in-memory only #[arg(long)] pub no_cache: bool, + + /// Seconds to allow the upload before giving up (default 300). + /// + /// A long session derives to several MB and goes up in one request, so + /// the ceiling that suits every other call is the wrong one here. Sets + /// `PATH_HTTP_TIMEOUT_SECS` for this invocation; export that directly for + /// commands that have no flag, or for an agent that runs `share` for you. + #[arg(long, value_name = "SECS")] + pub timeout: Option, } /// One artifact surfaced by a provider — today always an agent session. @@ -482,6 +491,15 @@ fn collect_cursor( } pub fn run(args: ShareArgs) -> Result<()> { + // Set before any client is built: `http_timeout` reads this, and every + // pathbase call in this process goes through one client constructor. Doing + // it here rather than threading a Duration keeps the eight call sites — all + // of them small requests that never needed a knob — untouched. + if let Some(secs) = args.timeout { + // SAFETY: single-threaded startup, before any client or task exists. + unsafe { std::env::set_var("PATH_HTTP_TIMEOUT_SECS", secs.to_string()) }; + } + let harness = args.harness.map(|h| h.artifact_type()); if args.session.is_some() && harness.is_none() { @@ -579,6 +597,9 @@ pub fn run(args: ShareArgs) -> Result<()> { None }, no_cache: args.no_cache, + // Already applied to the environment by `run`, so this is only carried + // for completeness — the picker path uploads through the same client. + timeout: args.timeout, }; // Show the conversation title in the confirmation line; the session id // is opaque and doesn't help the user verify they picked the right From 1c900882a17f53eeb0ffa2713bf93ac0acda846b Mon Sep 17 00:00:00 2001 From: Robert DeLanghe <1240090+bdelanghe@users.noreply.github.com> Date: Fri, 28 Aug 2026 14:18:20 -0400 Subject: [PATCH 2/2] fix(share): set the new timeout field in the test helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ShareArgs` gained `timeout`, and the test helper that builds one was not updated, so the test target failed to compile: error[E0063]: missing field `timeout` in initializer of `cmd_share::ShareArgs` CI reported it as `FAIL: test` with 7/8 gates passing — the library and binary built fine, only the test build broke, which is why it was easy to miss locally. None rather than a value: `run` applies this to the environment before any client is constructed, so it is never read on the path these tests take. Asserting against a knob they do not exercise would be theatre. --- crates/path-cli/src/cmd_share.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/path-cli/src/cmd_share.rs b/crates/path-cli/src/cmd_share.rs index 9a762bfe..e1750bf4 100644 --- a/crates/path-cli/src/cmd_share.rs +++ b/crates/path-cli/src/cmd_share.rs @@ -1429,6 +1429,11 @@ mod tests { session: None, project: None, no_cache: false, + // `run` applies this to the environment before any client exists, + // so it is never read on this path; None keeps these tests on the + // default timeout rather than asserting against a knob they do not + // exercise. + timeout: None, } }