From e5c3112c434938f227bd652a6fa62316aa4e9d91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hu=C3=A2n=20L=C3=AA-V=C6=B0=C6=A1ng?= <65440815+lehuan5062@users.noreply.github.com> Date: Sat, 11 Jul 2026 19:24:59 +0700 Subject: [PATCH 1/5] fix: add timeouts to remote resolution in status read path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add 5-second connect timeouts to gRPC and QUIC transports, and time-box the remote branch resolution in status() to 3 seconds. When the configured remote is unreachable, status calls now degrade gracefully instead of stalling for the full idle timeout (~30s for QUIC, 10s+ for retries). Critical fixes: 1. Time-box the entire remote resolution operation (both repository.remote() and branch::load_remote()), not just the fast path. This ensures the 3-second deadline covers the actual RPC calls that can hang indefinitely, even when the remote connection is already warm. 2. Fix available flag semantics: distinguish between "remote not reachable" (available=false) and "remote reachable but query failed" (available=true). A server that responds with a permission error or other non-branch-not-found error is still reachable and should not be reported as unavailable to users. Changes: - lore-transport: gRPC endpoint gets connect_timeout for initial connection - lore-revision: extract resolve_remote_latest() helper that returns (latest, authorized, available) where available reflects connection success rather than query success; eliminate double remote() call on event emission - tests: replace overfitted tautological test with two real unit tests Signed-off-by: Huân Lê-Vương <65440815+lehuan5062@users.noreply.github.com> --- lore-revision/src/repository/status.rs | 126 +++++++++++++++++++++++-- lore-transport/src/grpc/mod.rs | 3 + lore-transport/src/quic/client.rs | 15 ++- 3 files changed, 132 insertions(+), 12 deletions(-) diff --git a/lore-revision/src/repository/status.rs b/lore-revision/src/repository/status.rs index 00ade2b3..3a10184d 100644 --- a/lore-revision/src/repository/status.rs +++ b/lore-revision/src/repository/status.rs @@ -806,6 +806,55 @@ async fn count_subtrees(roots: Vec) -> Result<(u64, u64), StatusError Ok((directories, files)) } +const REMOTE_RESOLVE_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(3); + +/// Time-boxed remote branch resolution that degrades gracefully on timeout. +/// +/// Resolves the latest revision and authorization status of a branch on the +/// configured remote, timing out after the given deadline. On timeout or remote +/// unavailability, degrades to (None, false, false) — same shape as NoRemote — +/// ensuring an unreachable remote never stalls a local status read. +/// +/// # Arguments +/// +/// * `repository` — repository context with the configured remote. +/// * `branch_id` — the branch to query on the remote. +/// * `deadline` — maximum time to wait for remote resolution. +/// +/// # Returns +/// +/// A 3-tuple `(latest, authorized, available)` where: +/// - `latest` is the remote branch's latest revision hash, or None if unresolved. +/// - `authorized` is true iff the remote query returned an authoritative answer. +/// - `available` is true iff the remote connected and responded within the deadline +/// (even if with a non-branch-not-found error; a reachable server that errors is not unavailable). +async fn resolve_remote_latest( + repository: &Arc, + branch_id: BranchId, + deadline: std::time::Duration, +) -> (Option, bool, bool) { + async fn inner( + repository: &Arc, + branch_id: BranchId, + ) -> (Option, bool, bool) { + let remote = match repository.remote().await { + Ok(r) => r, + Err(_) => return (None, false, false), + }; + + match branch::load_remote(remote, repository.id, branch_id).await { + Ok(status) => (Some(status.latest), true, true), + Err(err) if err.is_branch_not_found() => (None, true, true), + Err(_) => (None, false, true), + } + } + + match tokio::time::timeout(deadline, inner(repository, branch_id)).await { + Ok((latest, authorized, available)) => (latest, authorized, available), + Err(_) => (None, false, false), + } +} + pub async fn status( repository: Arc, paths: Option>, @@ -907,14 +956,8 @@ pub async fn status( // Authorized only on an authoritative answer — a latest revision // or branch not found; proving the identity is authorized and has access - let (remote_latest, remote_authorized) = match repository.remote().await { - Ok(remote) => match branch::load_remote(remote, repository.id, branch.id).await { - Ok(status) => (Some(status.latest), true), - Err(err) if err.is_branch_not_found() => (None, true), - Err(_) => (None, false), - }, - Err(_) => (None, false), - }; + let (remote_latest, remote_authorized, remote_available) = + resolve_remote_latest(&repository, branch.id, REMOTE_RESOLVE_TIMEOUT).await; let remote_state = if let Some(remote_latest) = remote_latest { state::State::deserialize(repository.clone(), remote_latest) @@ -1080,7 +1123,7 @@ pub async fn status( }, local_ahead, remote_ahead, - repository.remote().await.is_ok(), + remote_available, remote_authorized, remote_latest.is_some(), ); @@ -1458,3 +1501,68 @@ pub async fn status( Ok(()) } + +#[cfg(test)] +mod remote_resolve_tests { + use std::future::pending; + + use futures::FutureExt; + + use super::*; + use crate::lore::BranchId; + use crate::repository::RemoteState; + use crate::repository::RepositoryContext; + use crate::repository::RepositoryFormat; + use crate::repository::create_client_memory_stores; + + async fn context_with_state(state: RemoteState) -> Arc { + let (immutable, mutable) = create_client_memory_stores() + .await + .expect("in-memory stores should be creatable"); + Arc::new(RepositoryContext::new_with_state( + None, + immutable, + mutable, + crate::lore::RepositoryId::default(), + crate::instance::InstanceId::default(), + state, + Arc::default(), + RepositoryFormat::Lore, + )) + } + + #[tokio::test] + async fn hung_remote_resolution_times_out_and_degrades() { + let never: futures::future::BoxFuture<'static, _> = pending().boxed(); + let shared = never.shared(); + let ctx = context_with_state(RemoteState::Pending(shared)).await; + + let result = resolve_remote_latest( + &ctx, + BranchId::default(), + std::time::Duration::from_millis(10), + ) + .await; + + assert_eq!( + result, + (None, false, false), + "timeout should degrade to unavailable" + ); + } + + #[tokio::test] + async fn offline_remote_resolves_to_unavailable() { + let ctx = context_with_state(RemoteState::Offline).await; + + let result = + resolve_remote_latest(&ctx, BranchId::default(), std::time::Duration::from_secs(3)) + .await; + + assert_eq!( + result, + (None, false, false), + "offline should degrade to unavailable" + ); + } +} diff --git a/lore-transport/src/grpc/mod.rs b/lore-transport/src/grpc/mod.rs index d5dad15f..d26b8a5e 100644 --- a/lore-transport/src/grpc/mod.rs +++ b/lore-transport/src/grpc/mod.rs @@ -69,6 +69,7 @@ pub const REVISION_LIST_STRATEGY_HEADER: &str = "x-lore-revision-list-strategy"; const RETRY_START_BACKOFF_MS: u64 = 50; const RETRY_MAX_BACKOFF_MS: u64 = 10_000; const RETRY_MAX_ATTEMPTS: usize = 60; +const GRPC_CONNECT_TIMEOUT_SECS: u64 = 5; fn grpc_retry() -> crate::util::Retry { crate::util::retry( @@ -575,6 +576,8 @@ async fn connect_to_endpoint(remote: &str) -> Result { lore_trace!("Set user agent to {user_agent}"); + endpoint = endpoint.connect_timeout(Duration::from_secs(GRPC_CONNECT_TIMEOUT_SECS)); + // Silent propagation of connection errors let channel = endpoint .connect() diff --git a/lore-transport/src/quic/client.rs b/lore-transport/src/quic/client.rs index a4df2b2b..8125b2fd 100644 --- a/lore-transport/src/quic/client.rs +++ b/lore-transport/src/quic/client.rs @@ -70,6 +70,7 @@ pub struct EndpointConfig { const IDLE_TIMEOUT_MS: u32 = 30000; const KEEP_ALIVE_MS: u64 = 500; +const HANDSHAKE_TIMEOUT_SECS: u64 = 5; pub const DEFAULT_EXPECTED_RTT_MS: u64 = 100; #[derive(Clone, Debug)] @@ -772,14 +773,22 @@ pub async fn connect( Ok(mut endpoint) => { endpoint.set_default_client_config(client_config.clone()); match endpoint.connect(remote_addr, server_name) { - Ok(connecting) => match connecting.await { - Ok(connection) => { + Ok(connecting) => match tokio::time::timeout( + Duration::from_secs(HANDSHAKE_TIMEOUT_SECS), + connecting, + ) + .await + { + Ok(Ok(connection)) => { lore_debug!("Success QUIC connecting to {remote_addr}"); return Ok(connection); } - Err(err) => { + Ok(Err(err)) => { lore_debug!("Failed QUIC connecting to {remote_addr}: {err}"); } + Err(_) => { + lore_debug!("QUIC handshake timeout to {remote_addr}"); + } }, Err(err) => { lore_debug!("Failed QUIC connect to {remote_addr}: {err}"); From 0d146b9ba3616bc7f4c7bdd40bd74aa0c59ddb76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hu=C3=A2n=20L=C3=AA-V=C6=B0=C6=A1ng?= <65440815+lehuan5062@users.noreply.github.com> Date: Sat, 11 Jul 2026 22:05:35 +0700 Subject: [PATCH 2/5] fix: backtick NoRemote in resolve_remote_latest doc comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit clippy::doc_markdown (denied via -D warnings in the pre-commit hook) flags bare type-name references in doc comments. CI caught this; local clippy runs missed it because they weren't re-run after this doc comment was added in a later revision of the same change. Signed-off-by: Huân Lê-Vương <65440815+lehuan5062@users.noreply.github.com> --- lore-revision/src/repository/status.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lore-revision/src/repository/status.rs b/lore-revision/src/repository/status.rs index 3a10184d..54a69c6d 100644 --- a/lore-revision/src/repository/status.rs +++ b/lore-revision/src/repository/status.rs @@ -812,7 +812,7 @@ const REMOTE_RESOLVE_TIMEOUT: std::time::Duration = std::time::Duration::from_se /// /// Resolves the latest revision and authorization status of a branch on the /// configured remote, timing out after the given deadline. On timeout or remote -/// unavailability, degrades to (None, false, false) — same shape as NoRemote — +/// unavailability, degrades to (None, false, false) — same shape as `NoRemote` — /// ensuring an unreachable remote never stalls a local status read. /// /// # Arguments From 9a753eb08ee6694c9ded8f8594eaab3105f1036e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hu=C3=A2n=20L=C3=AA-V=C6=B0=C6=A1ng?= <65440815+lehuan5062@users.noreply.github.com> Date: Mon, 13 Jul 2026 20:53:13 +0700 Subject: [PATCH 3/5] fix: remove status-level timeout from remote resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop the 3-second status-level deadline from resolve_remote_latest and rely solely on transport-level connect timeouts (5s for gRPC handshake). The status-level timeout was layering two separate concerns and timing out slow lookups instead of bounding unreachable remotes. The root cause is QUIC's 30-second idle timeout on unreachable remotes: UDP offers no connection-refused signal, so every local status read would stall ~10–30s waiting for the timeout. Transport-level connect timeouts now bound this wait and are the single timeout layer. Update resolve_remote_latest to log previously-discarded errors at debug level before degrading gracefully. Remove the hung-remote timeout test (now invalid without a local timeout); add a failed-remote test mirroring terminal state from a bounded transport connect. Update doc comments to explain the degradation contract. Signed-off-by: Huân Lê-Vương <65440815+lehuan5062@users.noreply.github.com> --- lore-revision/src/repository/status.rs | 83 +++++++++++--------------- 1 file changed, 35 insertions(+), 48 deletions(-) diff --git a/lore-revision/src/repository/status.rs b/lore-revision/src/repository/status.rs index 54a69c6d..bd4f458f 100644 --- a/lore-revision/src/repository/status.rs +++ b/lore-revision/src/repository/status.rs @@ -806,52 +806,45 @@ async fn count_subtrees(roots: Vec) -> Result<(u64, u64), StatusError Ok((directories, files)) } -const REMOTE_RESOLVE_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(3); - -/// Time-boxed remote branch resolution that degrades gracefully on timeout. +/// Remote branch resolution that degrades gracefully on unavailability. /// /// Resolves the latest revision and authorization status of a branch on the -/// configured remote, timing out after the given deadline. On timeout or remote -/// unavailability, degrades to (None, false, false) — same shape as `NoRemote` — -/// ensuring an unreachable remote never stalls a local status read. +/// configured remote. On remote unavailability, degrades to (None, false, false) — +/// same shape as `NoRemote` — ensuring an unreachable remote never stalls a local +/// status read. Transport-level connect timeouts in lore-transport bound the wait; +/// status is a local read that reports remote state opportunistically. /// /// # Arguments /// /// * `repository` — repository context with the configured remote. /// * `branch_id` — the branch to query on the remote. -/// * `deadline` — maximum time to wait for remote resolution. /// /// # Returns /// /// A 3-tuple `(latest, authorized, available)` where: /// - `latest` is the remote branch's latest revision hash, or None if unresolved. /// - `authorized` is true iff the remote query returned an authoritative answer. -/// - `available` is true iff the remote connected and responded within the deadline +/// - `available` is true iff the remote connected and responded successfully /// (even if with a non-branch-not-found error; a reachable server that errors is not unavailable). async fn resolve_remote_latest( repository: &Arc, branch_id: BranchId, - deadline: std::time::Duration, ) -> (Option, bool, bool) { - async fn inner( - repository: &Arc, - branch_id: BranchId, - ) -> (Option, bool, bool) { - let remote = match repository.remote().await { - Ok(r) => r, - Err(_) => return (None, false, false), - }; - - match branch::load_remote(remote, repository.id, branch_id).await { - Ok(status) => (Some(status.latest), true, true), - Err(err) if err.is_branch_not_found() => (None, true, true), - Err(_) => (None, false, true), + let remote = match repository.remote().await { + Ok(r) => r, + Err(err) => { + lore_debug!("Remote unavailable for status: {err}"); + return (None, false, false); } - } + }; - match tokio::time::timeout(deadline, inner(repository, branch_id)).await { - Ok((latest, authorized, available)) => (latest, authorized, available), - Err(_) => (None, false, false), + match branch::load_remote(remote, repository.id, branch_id).await { + Ok(status) => (Some(status.latest), true, true), + Err(err) if err.is_branch_not_found() => (None, true, true), + Err(err) => { + lore_debug!("Remote branch query failed: {err}"); + (None, false, true) + } } } @@ -957,7 +950,7 @@ pub async fn status( // Authorized only on an authoritative answer — a latest revision // or branch not found; proving the identity is authorized and has access let (remote_latest, remote_authorized, remote_available) = - resolve_remote_latest(&repository, branch.id, REMOTE_RESOLVE_TIMEOUT).await; + resolve_remote_latest(&repository, branch.id).await; let remote_state = if let Some(remote_latest) = remote_latest { state::State::deserialize(repository.clone(), remote_latest) @@ -1504,17 +1497,20 @@ pub async fn status( #[cfg(test)] mod remote_resolve_tests { - use std::future::pending; - - use futures::FutureExt; + use lore_transport::ProtocolError; use super::*; + use crate::errors::Disconnected; use crate::lore::BranchId; use crate::repository::RemoteState; use crate::repository::RepositoryContext; use crate::repository::RepositoryFormat; use crate::repository::create_client_memory_stores; + fn disconnected() -> ProtocolError { + ProtocolError::from(Disconnected) + } + async fn context_with_state(state: RemoteState) -> Arc { let (immutable, mutable) = create_client_memory_stores() .await @@ -1532,37 +1528,28 @@ mod remote_resolve_tests { } #[tokio::test] - async fn hung_remote_resolution_times_out_and_degrades() { - let never: futures::future::BoxFuture<'static, _> = pending().boxed(); - let shared = never.shared(); - let ctx = context_with_state(RemoteState::Pending(shared)).await; - - let result = resolve_remote_latest( - &ctx, - BranchId::default(), - std::time::Duration::from_millis(10), - ) - .await; + async fn offline_remote_resolves_to_unavailable() { + let ctx = context_with_state(RemoteState::Offline).await; + + let result = resolve_remote_latest(&ctx, BranchId::default()).await; assert_eq!( result, (None, false, false), - "timeout should degrade to unavailable" + "offline should degrade to unavailable" ); } #[tokio::test] - async fn offline_remote_resolves_to_unavailable() { - let ctx = context_with_state(RemoteState::Offline).await; + async fn failed_remote_resolves_to_unavailable() { + let ctx = context_with_state(RemoteState::Failed(disconnected())).await; - let result = - resolve_remote_latest(&ctx, BranchId::default(), std::time::Duration::from_secs(3)) - .await; + let result = resolve_remote_latest(&ctx, BranchId::default()).await; assert_eq!( result, (None, false, false), - "offline should degrade to unavailable" + "failed remote should degrade to unavailable" ); } } From 9960b0b549c11df0e17e0488bf556cd629da2ea6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hu=C3=A2n=20L=C3=AA-V=C6=B0=C6=A1ng?= <65440815+lehuan5062@users.noreply.github.com> Date: Tue, 14 Jul 2026 16:17:52 +0700 Subject: [PATCH 4/5] docs: trim resolve_remote_latest doc comment to house style MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lore's comment standard calls for minimal, purposeful doc comments and no restating of self-explanatory code. The rustdoc # Arguments/# Returns sections here described obvious parameters (repository, branch_id) and are rare elsewhere in the crate. Collapse to a terse summary that keeps only the non-obvious facts: graceful NoRemote degradation and the available flag reflecting connectivity rather than query success. Signed-off-by: Huân Lê-Vương <65440815+lehuan5062@users.noreply.github.com> --- lore-revision/src/repository/status.rs | 24 +++++------------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/lore-revision/src/repository/status.rs b/lore-revision/src/repository/status.rs index bd4f458f..213ce95b 100644 --- a/lore-revision/src/repository/status.rs +++ b/lore-revision/src/repository/status.rs @@ -806,26 +806,12 @@ async fn count_subtrees(roots: Vec) -> Result<(u64, u64), StatusError Ok((directories, files)) } -/// Remote branch resolution that degrades gracefully on unavailability. +/// Resolve the remote's latest revision for a branch, degrading to the existing +/// `NoRemote` state when the remote is unavailable so an unreachable remote never +/// stalls a local status read; transport connect timeouts bound the wait. /// -/// Resolves the latest revision and authorization status of a branch on the -/// configured remote. On remote unavailability, degrades to (None, false, false) — -/// same shape as `NoRemote` — ensuring an unreachable remote never stalls a local -/// status read. Transport-level connect timeouts in lore-transport bound the wait; -/// status is a local read that reports remote state opportunistically. -/// -/// # Arguments -/// -/// * `repository` — repository context with the configured remote. -/// * `branch_id` — the branch to query on the remote. -/// -/// # Returns -/// -/// A 3-tuple `(latest, authorized, available)` where: -/// - `latest` is the remote branch's latest revision hash, or None if unresolved. -/// - `authorized` is true iff the remote query returned an authoritative answer. -/// - `available` is true iff the remote connected and responded successfully -/// (even if with a non-branch-not-found error; a reachable server that errors is not unavailable). +/// Returns `(latest, authorized, available)`, where `available` reflects +/// connectivity, not query success — a reachable remote that errors is still available. async fn resolve_remote_latest( repository: &Arc, branch_id: BranchId, From 6fc2962da629e47066cb5453b175fb8aeb7d3775 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hu=C3=A2n=20L=C3=AA-V=C6=B0=C6=A1ng?= <65440815+lehuan5062@users.noreply.github.com> Date: Thu, 16 Jul 2026 19:58:46 +0700 Subject: [PATCH 5/5] lore-revision: Skip needless remote connects in local read paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Several read verbs drove a remote connect even when local data is what gets returned, stalling for the bounded connect timeout against an unreachable server: - revision::resolve drove the connect before checking should_search_remote, so branch@LATEST under --local/--offline connected anyway; reorder the guard to match the @number path. - Explicit-branch revision history connected unconditionally and then discarded the remote latest unless --remote was set; move the fetch inside the --remote branch. - File history connected before taking the --local early-return; compute the local latest and take the early-return first. - auth::resolve_user_info (driven by the CLI after history listings to prettify user ids) connected regardless of --local/--offline; skip resolution there and fall back to raw ids. With a killed server, revision history --local drops from ~4.1s to ~150ms; default-mode explicit-branch history no longer wastes a connect in the history lookup itself. Guarded by a new Python integration test against a killed server and a Rust unit test for the local-only branch@LATEST resolve. Signed-off-by: Huân Lê-Vương <65440815+lehuan5062@users.noreply.github.com> --- lore-revision/src/auth/userinfo.rs | 12 ++ lore-revision/src/file/history.rs | 14 +- lore-revision/src/revision.rs | 4 +- lore-revision/src/revision/history.rs | 15 +- lore-revision/tests/revision.rs | 22 +++ scripts/test/test_local_reads_skip_connect.py | 140 ++++++++++++++++++ 6 files changed, 189 insertions(+), 18 deletions(-) create mode 100644 scripts/test/test_local_reads_skip_connect.py diff --git a/lore-revision/src/auth/userinfo.rs b/lore-revision/src/auth/userinfo.rs index 2e24638f..db1806e9 100644 --- a/lore-revision/src/auth/userinfo.rs +++ b/lore-revision/src/auth/userinfo.rs @@ -91,10 +91,22 @@ pub struct LoreAuthIdentityEventData { /// remote path. Because the local path reads a cached token captured at /// login, a name change made server-side since then is not reflected until /// the token is refreshed. +/// +/// In offline or local-only mode, returns without emitting any events — +/// resolution requires the remote auth service, and callers fall back to +/// displaying the raw ids. pub async fn resolve_user_info( repository: Arc, ids: LoreArray, ) -> Result<(), UserInfoError> { + // Resolution needs the remote auth service; a local-only or offline + // operation must not drive a connect just to prettify display names. + // Callers fall back to showing the raw ids. + if execution_context().globals().offline_or_local() { + lore_debug!("Skipping user info resolution in offline/local mode"); + return Ok(()); + } + let remote = repository .remote() .await diff --git a/lore-revision/src/file/history.rs b/lore-revision/src/file/history.rs index e11effcd..515cc229 100644 --- a/lore-revision/src/file/history.rs +++ b/lore-revision/src/file/history.rs @@ -374,6 +374,13 @@ async fn find_start_revision( resolved.id }; + let local_latest = branch::load_latest(repository.clone(), branch) + .await + .unwrap_or_default(); + if execution_context().globals().local() { + return Ok(local_latest); + } + let remote_latest = if let Ok(remote) = repository.remote().await { branch::load_remote_latest(remote.clone(), repository.id, branch) .await @@ -385,13 +392,6 @@ async fn find_start_revision( return Ok(remote_latest); } - let local_latest = branch::load_latest(repository.clone(), branch) - .await - .unwrap_or_default(); - if execution_context().globals().local() { - return Ok(local_latest); - } - // Is there a latest? if remote_latest.is_zero() { return Ok(local_latest); diff --git a/lore-revision/src/revision.rs b/lore-revision/src/revision.rs index 395e59ba..c9cd8eab 100644 --- a/lore-revision/src/revision.rs +++ b/lore-revision/src/revision.rs @@ -1219,9 +1219,7 @@ pub async fn resolve( branch_status.id }; - let remote_latest = if let Ok(remote) = repository.remote().await - && should_search_remote - { + let remote_latest = if should_search_remote && let Ok(remote) = repository.remote().await { branch::load_remote_latest(remote.clone(), repository.id, branch) .await .ok() diff --git a/lore-revision/src/revision/history.rs b/lore-revision/src/revision/history.rs index 81dd2145..9d577e58 100644 --- a/lore-revision/src/revision/history.rs +++ b/lore-revision/src/revision/history.rs @@ -177,15 +177,14 @@ async fn find_start_revision( .await .internal("loading branch name")?; - let remote_latest = if let Ok(remote) = repository.remote().await { - branch::load_remote_latest(remote.clone(), repository.id, branch) - .await - .unwrap_or_default() - } else { - Hash::default() - }; - if execution_context().globals().remote() { + let remote_latest = if let Ok(remote) = repository.remote().await { + branch::load_remote_latest(remote.clone(), repository.id, branch) + .await + .unwrap_or_default() + } else { + Hash::default() + }; return Ok((remote_latest, Some(branch))); } diff --git a/lore-revision/tests/revision.rs b/lore-revision/tests/revision.rs index 8aea3344..e951c492 100644 --- a/lore-revision/tests/revision.rs +++ b/lore-revision/tests/revision.rs @@ -106,6 +106,28 @@ mod tests { .await; } + // Exercises the branch@LATEST arm with a local-only search location: the + // remote must not be consulted and the miss surfaces as RevisionNotFound. + #[tokio::test] + async fn resolve_latest_local_only_returns_revision_not_found_without_remote() { + let execution = setup_test_execution(); + LORE_CONTEXT + .scope(execution, async move { + let repository = make_repo_context().await; + let signature = format!("{}@LATEST", uuid::Uuid::now_v7()); + let err = + revision::resolve(repository, signature, None, ResolveSearchLocation::Local) + .await + .expect_err("resolve should fail for unknown branch latest"); + assert!( + err.is_revision_not_found(), + "expected RevisionNotFound, got {err:?}" + ); + assert!(!err.is_internal()); + }) + .await; + } + #[tokio::test] async fn test_diff3() { let (_immutable_store, _mutable_store, execution) = diff --git a/scripts/test/test_local_reads_skip_connect.py b/scripts/test/test_local_reads_skip_connect.py new file mode 100644 index 00000000..99ccbed6 --- /dev/null +++ b/scripts/test/test_local_reads_skip_connect.py @@ -0,0 +1,140 @@ +# SPDX-FileCopyrightText: 2026 Epic Games, Inc. +# SPDX-License-Identifier: MIT +"""Regression test: --local read verbs must not stall on a remote connect +when the server is unreachable. + +Follow-up to the remote-resolution-timeout fix, which bounded the connect +stall with 5 s transport timeouts but left several read paths driving a +remote connect whose result is never used in the --local case: + +- ``revision::resolve`` drove the connect before checking + ``should_search_remote``, so ``branch@LATEST`` under --local/--offline + connected anyway. +- explicit-branch ``revision history`` connected unconditionally and then + discarded the remote latest unless --remote was set. +- ``file history`` connected before taking the --local early-return. +- ``auth::resolve_user_info`` (driven by the CLI after every history + listing to prettify user ids) connected regardless of --local. + +With those paths fixed, each --local read below must return the local +latest promptly against a killed server. Before the fix each call paid a +bounded multi-second connect timeout; the threshold sits well under that. + +The default (no-flags) read is value-checked but only loosely bounded: in +default mode the CLI's user-info resolution may still legitimately try the +remote and pay the bounded connect timeout when the server is down. +""" + +import logging +import time + +import pytest + +from lore_server import ( + allocate_free_port, + generate_server_config, + launch_lore_server, +) +from test_branch_switch_reconnect import _force_kill_server + +logger = logging.getLogger(__name__) + +# --local reads must complete well under the ~5 s transport connect timeout +# the pre-fix code paid against an unreachable server. +LOCAL_READ_DEADLINE_S = 3.0 + +# The default read may pay one bounded connect timeout (user-info +# resolution) but must not stack several of them. +DEFAULT_READ_DEADLINE_S = 8.0 + + +def _timed(label: str, deadline_s: float, fn): + start = time.monotonic() + result = fn() + elapsed = time.monotonic() - start + logger.info("%s completed in %.2fs", label, elapsed) + assert elapsed < deadline_s, ( + f"{label} took {elapsed:.2f}s against a killed server — " + "it is still driving a needless remote connect" + ) + return result + + +@pytest.mark.smoke +def test_local_reads_skip_remote_connect( + request, + tmp_path_factory, + lore_server_executable_path, + new_lore_repo, +): + # Dedicated server for this test so killing it doesn't disrupt tests + # that share the session-scoped autouse server. Mirrors the pattern in + # scripts/test/test_branch_switch_reconnect.py. + shared_port = allocate_free_port() + server_ports = { + "quic": shared_port, + "grpc": shared_port, + "http": allocate_free_port(), + "internal": allocate_free_port(), + } + server_root, server_env = generate_server_config( + request, tmp_path_factory, server_ports + ) + server_proc, _server_log_path, server_log_fd = launch_lore_server( + server_root, server_env, lore_server_executable_path + ) + try: + repo = new_lore_repo(remote_url=f"lore://127.0.0.1:{server_ports['quic']}/") + text_file = "file.txt" + repo.write_commit_push("Initial commit", {text_file: ["Line one\n"]}) + local_latest = repo.revision_history(1, offline=True)[0].signature + finally: + # Kill the server outright: every read below must succeed on local + # data alone, without waiting out remote connect timeouts. + _force_kill_server(server_proc, server_log_fd) + + # Explicit-branch history under --local: the search-location contract + # forbids any remote traffic, so the read must be near-instant. + revisions = _timed( + "history(branch=main, --local)", + LOCAL_READ_DEADLINE_S, + lambda: repo.history(1, branch="main", local=True), + ) + assert revisions and revisions[0].signature == local_latest, ( + "--local explicit-branch history must return the local latest" + ) + + # branch@LATEST resolve under --local exercises revision::resolve, + # which previously drove the connect before checking the search + # location. + revisions = _timed( + "history(revision=main@LATEST, --local)", + LOCAL_READ_DEADLINE_S, + lambda: repo.history(1, revision="main@LATEST", local=True), + ) + assert revisions and revisions[0].signature == local_latest, ( + "--local branch@LATEST resolve must return the local latest" + ) + + # File history under --local previously connected before taking the + # --local early-return. + output = _timed( + "file history(--local)", + LOCAL_READ_DEADLINE_S, + lambda: repo.file_history(text_file, branch="main", local=True), + ) + assert "Initial commit" in output, ( + "--local file history must list the committing revision" + ) + + # Explicit-branch history with no flags returns the local latest + # without the history lookup itself connecting; only the CLI's + # user-info resolution may still pay one bounded connect timeout. + revisions = _timed( + "history(branch=main)", + DEFAULT_READ_DEADLINE_S, + lambda: repo.history(1, branch="main"), + ) + assert revisions and revisions[0].signature == local_latest, ( + "default explicit-branch history must return the local latest" + )