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/repository/status.rs b/lore-revision/src/repository/status.rs index 00ade2b3..213ce95b 100644 --- a/lore-revision/src/repository/status.rs +++ b/lore-revision/src/repository/status.rs @@ -806,6 +806,34 @@ async fn count_subtrees(roots: Vec) -> Result<(u64, u64), StatusError Ok((directories, files)) } +/// 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. +/// +/// 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, +) -> (Option, bool, bool) { + let remote = match repository.remote().await { + Ok(r) => r, + Err(err) => { + lore_debug!("Remote unavailable for status: {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(err) => { + lore_debug!("Remote branch query failed: {err}"); + (None, false, true) + } + } +} + pub async fn status( repository: Arc, paths: Option>, @@ -907,14 +935,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).await; let remote_state = if let Some(remote_latest) = remote_latest { state::State::deserialize(repository.clone(), remote_latest) @@ -1080,7 +1102,7 @@ pub async fn status( }, local_ahead, remote_ahead, - repository.remote().await.is_ok(), + remote_available, remote_authorized, remote_latest.is_some(), ); @@ -1458,3 +1480,62 @@ pub async fn status( Ok(()) } + +#[cfg(test)] +mod remote_resolve_tests { + 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 + .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 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), + "offline should degrade to unavailable" + ); + } + + #[tokio::test] + async fn failed_remote_resolves_to_unavailable() { + let ctx = context_with_state(RemoteState::Failed(disconnected())).await; + + let result = resolve_remote_latest(&ctx, BranchId::default()).await; + + assert_eq!( + result, + (None, false, false), + "failed remote should degrade to unavailable" + ); + } +} 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/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}"); 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" + )