From f494baa42d6794a10464accb6b2e305f586a222f Mon Sep 17 00:00:00 2001 From: Mattias Jansson Date: Mon, 20 Jul 2026 14:34:11 +0200 Subject: [PATCH 1/6] Resolve relative paths against the caller's working directory # Summary A command sent to the service resolved its relative paths in the service process, against whatever working directory the service happened to inherit from whoever started it. `lore clone ` from the home directory put the repository under the directory the service was started in instead, because the CLI passes the name derived from the URL as a relative repository path and `make_absolute` joined it to the service's own working directory. This affects every relative path a command carries, not just clone's destination: `make_absolute` is the single point where relative paths are resolved, and each of its callers runs in the service when the call is routed there. ## Changes - `LoreGlobalArgs` gains `working_directory`, which already crosses the wire with the rest of the globals. The CLI fills it with the directory the command was run in. - `make_absolute` resolves against that directory when the call in progress named one, and against this process's own otherwise, so a call executed locally behaves exactly as before. Resolution is per call rather than process state, which matters because the service handles connections concurrently and a shared working directory would let two commands corrupt each other. - `make_absolute_from` takes the base directory explicitly, for the call wrappers that resolve paths before the execution context exists and so cannot look it up. `prepare_repository_call` is one: it absolutizes the repository path before `setup_execution`, and would otherwise still have used the service's directory. - The service now parks its working directory at the filesystem root, rather than keeping whatever it inherited. Callers send the directory their relative paths belong to, so it needs none of its own, and not holding one also keeps it from pinning a filesystem it has no interest in. # Test Plan With the service started from one directory and commands run from another, verified on macOS that both resolution paths now use the caller's directory: - Through the explicit base: `--repository relsub status` over IPC reports the same absolute path as the identical call executed locally. - Through the context lookup, which is the path clone takes: `repository create` with a relative repository path created the repository under the client's directory, and not under the service's. - The service's working directory is the filesystem root, confirmed with `lsof -d cwd`. - `cargo clippy --all-targets -- -D warnings --no-deps`, `cargo +nightly fmt --all --check`, `cargo test -p lore --lib` (90), `cargo test -p lore-revision --lib` (159): all clean. Not verified on Windows, including the `SystemRoot` working directory. Signed-off-by: Mattias Jansson --- lore-capi/lore.h | 6 ++++ lore-client/src/cli/cli.rs | 4 +++ lore-client/src/cli/commands/service/run.rs | 27 ++++++++++++++ lore-revision/src/interface.rs | 10 ++++++ lore-revision/src/util/path.rs | 40 ++++++++++++++++----- lore/src/call.rs | 16 +++++---- 6 files changed, 87 insertions(+), 16 deletions(-) diff --git a/lore-capi/lore.h b/lore-capi/lore.h index e65c1406..f04f7eb6 100644 --- a/lore-capi/lore.h +++ b/lore-capi/lore.h @@ -3530,6 +3530,12 @@ typedef struct lore_event_t { typedef struct lore_global_args_t { // Repository path struct lore_string_t repository_path; + // Directory that relative paths in this call are resolved against. Set it + // when a call may be executed by another process, such as the Lore + // service, whose own working directory is unrelated to the caller's. When + // empty, relative paths resolve against the working directory of the + // process performing the call. + struct lore_string_t working_directory; // Correlation ID struct lore_string_t correlation_id; // Identity to use diff --git a/lore-client/src/cli/cli.rs b/lore-client/src/cli/cli.rs index 076078cc..245aa68e 100644 --- a/lore-client/src/cli/cli.rs +++ b/lore-client/src/cli/cli.rs @@ -349,6 +349,10 @@ pub enum LoreCliError { pub fn lore_globals_from_args(cli: &LoreCli) -> LoreGlobalArgs { let mut args = LoreGlobalArgs { repository_path: get_repository_path(cli.repository.clone()), + working_directory: std::env::current_dir() + .map(|path| path.display().to_string()) + .unwrap_or_default() + .into(), force: cli.force.into(), dry_run: cli.dry_run.into(), diff --git a/lore-client/src/cli/commands/service/run.rs b/lore-client/src/cli/commands/service/run.rs index e0f6717c..e6bc9656 100644 --- a/lore-client/src/cli/commands/service/run.rs +++ b/lore-client/src/cli/commands/service/run.rs @@ -35,6 +35,25 @@ pub enum ServiceMainError {} /// left behind is a stale socket, which the next start detects and removes. const SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(5); +/// Where the service parks its working directory. It inherits one from whoever +/// started it, which is unrelated to the directories its callers run in, and +/// holding that directory would also keep the filesystem under it busy. Callers +/// send the directory their relative paths belong to, so the service never +/// needs one of its own. +fn detached_working_directory() -> std::path::PathBuf { + #[cfg(target_family = "unix")] + { + std::path::PathBuf::from("/") + } + #[cfg(target_os = "windows")] + { + std::env::var_os("SystemRoot").map_or_else( + || std::path::PathBuf::from("C:\\"), + std::path::PathBuf::from, + ) + } +} + pub async fn service_main( listening_signal: Option>, ) -> Result<(), ServiceMainError> { @@ -42,6 +61,14 @@ pub async fn service_main( return Err(ServiceMainError::internal("IPC not supported on this OS")); } + let detached = detached_working_directory(); + if let Err(error) = std::env::set_current_dir(&detached) { + eprintln!( + "Failed to set working directory to {}: {error}", + detached.display() + ); + } + let listener: UdsListener = UdsListener::new().internal("Failed to start listener socket")?; if let Some(listening_signal) = listening_signal { diff --git a/lore-revision/src/interface.rs b/lore-revision/src/interface.rs index d4a4b394..2266b64d 100644 --- a/lore-revision/src/interface.rs +++ b/lore-revision/src/interface.rs @@ -527,6 +527,12 @@ pub enum LoreLoadConfig { pub struct LoreGlobalArgs { /// Repository path pub repository_path: LoreString, + /// Directory that relative paths in this call are resolved against. Set it + /// when a call may be executed by another process, such as the Lore + /// service, whose own working directory is unrelated to the caller's. When + /// empty, relative paths resolve against the working directory of the + /// process performing the call. + pub working_directory: LoreString, /// Correlation ID pub correlation_id: LoreString, /// Identity to use @@ -579,6 +585,10 @@ impl LoreGlobalArgs { self.repository_path.as_str() } + pub fn working_directory(&self) -> Option<&str> { + (&self.working_directory).into() + } + pub fn identity(&self) -> Option<&str> { (&self.identity).into() } diff --git a/lore-revision/src/util/path.rs b/lore-revision/src/util/path.rs index 1ccb80de..477156e0 100644 --- a/lore-revision/src/util/path.rs +++ b/lore-revision/src/util/path.rs @@ -16,21 +16,43 @@ pub enum PathError { InvalidPath, } +/// The directory the call in progress resolves relative paths against, when it +/// named one. A call arriving over IPC carries the directory of the process +/// that made it, because the service's own is unrelated to the caller's. +fn call_working_directory() -> Option { + let context = crate::runtime::try_execution_context()?; + let directory = context.globals().working_directory()?; + Some(PathBuf::from(directory)) +} + +/// Resolves `path` against the working directory of the call in progress, or +/// against this process's own when the call did not name one. pub fn make_absolute(path: impl AsRef) -> Result { + make_absolute_from(path, call_working_directory()) +} + +/// [`make_absolute`] with the base directory supplied by the caller, for the +/// call wrappers that resolve paths before the execution context exists and so +/// cannot look it up. +pub fn make_absolute_from( + path: impl AsRef, + base: Option, +) -> Result { let path = path.as_ref(); let cleanpath = clean(path.to_owned()); let pathbuf = PathBuf::from_str(cleanpath.as_str()).emit_map_err(InvalidPath { path: path.to_string(), })?; - if !pathbuf.is_absolute() { - Ok(std::env::current_dir() - .emit_map_err(PathError::internal( - "failed to get current working directory", - ))? - .join(pathbuf)) - } else { - Ok(pathbuf) - } + if pathbuf.is_absolute() { + return Ok(pathbuf); + } + let base = match base { + Some(base) => base, + None => std::env::current_dir().emit_map_err(PathError::internal( + "failed to get current working directory", + ))?, + }; + Ok(base.join(pathbuf)) } /// Returns `true` when `candidate` resolves to a location inside diff --git a/lore/src/call.rs b/lore/src/call.rs index ff2e7ba7..8106e8a7 100644 --- a/lore/src/call.rs +++ b/lore/src/call.rs @@ -221,13 +221,15 @@ async fn prepare_repository_call( mut globals: LoreGlobalArgs, callback: LoreEventCallback, ) -> Result<(PathBuf, Arc), i32> { - let repository_path = - if let Ok(path) = util::path::make_absolute(globals.repository_path.as_str()) { - globals.repository_path = path.display().to_string().into(); - path - } else { - PathBuf::from(globals.repository_path.as_str()) - }; + let working_directory = globals.working_directory().map(PathBuf::from); + let repository_path = if let Ok(path) = + util::path::make_absolute_from(globals.repository_path.as_str(), working_directory) + { + globals.repository_path = path.display().to_string().into(); + path + } else { + PathBuf::from(globals.repository_path.as_str()) + }; let execution = setup_execution(globals, callback); From 45e42512f8afb9aa69d2399ba2a7e04cb903f402 Mon Sep 17 00:00:00 2001 From: Mattias Jansson Date: Mon, 20 Jul 2026 14:40:09 +0200 Subject: [PATCH 2/6] Route file write output through the call's working directory # Summary An audit of working-directory use across the workspace found two more places in the library that resolve a relative path against the process working directory, which is the service's rather than the caller's when the call is routed there. Both open-code the same join that `make_absolute` performs, so they were not covered by teaching `make_absolute` to use the directory the call carries. ## Changes - `lore-revision/src/file/write.rs`: resolve the output path with `make_absolute` instead of joining `std::env::current_dir` directly, in both places that did so. Writing to a relative output path through the service now writes where the caller asked, rather than under the service's directory. - `clippy.toml`: ban `std::env::current_dir` so the library cannot reacquire this class of bug. See the caveat in the test plan: the file is not currently read, so the entry does not yet take effect. - `make_absolute_from` takes the base directory as a `&Path` rather than an owned `PathBuf`, and `make_absolute` borrows it from the execution context it already holds for the duration of the call. Resolving a relative path no longer allocates a copy of the directory to do so. The remaining uses are deliberate and stay: - `make_absolute` falls back to the process directory when the call does not carry one, which is what a caller executing in its own process wants. - The CLI reads the process directory to find the repository root, to render paths relative for display, and to fill in the directory it sends with each call. Those all run in the process the user invoked, which is the directory they mean. - The service sets its own working directory once at start-up, deliberately. - `lore-server` reads it when resolving its config path, in its own process. # Test Plan - `cargo build -p lore-revision` and `cargo test -p lore-revision --lib` (159 passed): clean. Caveat on the clippy ban: `clippy.toml` at the workspace root is not read for workspace members, which was confirmed by putting invalid TOML in it and seeing clippy report nothing. The existing bans in that file have therefore never been enforced either. Making them take effect needs the config pointed at explicitly, which will surface pre-existing violations of the other bans and is left as a separate change. Known remaining issue, not addressed here: `lore-revision/src/projfs/serve.rs` sets the process working directory while serving a repository. That is process global state in a process that serves several callers at once, so it would disturb concurrent calls. It predates this work and warrants its own change. Signed-off-by: Mattias Jansson --- clippy.toml | 1 + lore-revision/src/file/write.rs | 8 ++++---- lore-revision/src/util/path.rs | 36 ++++++++++++++++----------------- lore/src/call.rs | 9 +++++---- 4 files changed, 28 insertions(+), 26 deletions(-) diff --git a/clippy.toml b/clippy.toml index f5453a26..c84fd9b6 100644 --- a/clippy.toml +++ b/clippy.toml @@ -1,4 +1,5 @@ disallowed-methods = [ + { path = "std::env::current_dir", reason = "The library must not depend on the process working directory: a call executed by the Lore service runs in a process whose directory is unrelated to the caller's. Use lore_revision::util::path::make_absolute, which resolves against the working directory the call carries." }, { path = "dashmap::DashMap::entry", reason = "This function internally takes a write lock and returns it in the entry handle. This can potentially block worker threads and cause deadlocks if used incorrectly. If you're sure it's ok add #[allow(clippy::disallowed_methods)] with a comment explaining why its safe" }, { path = "tokio::spawn", reason = "Use lore_base::lore_spawn! so LORE_CONTEXT is propagated to the spawned task." }, { path = "tokio::task::spawn", reason = "Use lore_base::lore_spawn! so LORE_CONTEXT is propagated to the spawned task." }, diff --git a/lore-revision/src/file/write.rs b/lore-revision/src/file/write.rs index 0f676a70..d69ab696 100644 --- a/lore-revision/src/file/write.rs +++ b/lore-revision/src/file/write.rs @@ -167,14 +167,14 @@ pub async fn write_file( let destination = { let mut absolute_path = Path::new(&output).to_path_buf(); if !absolute_path.is_absolute() { - let Ok(current_path) = std::env::current_dir() else { + let Ok(resolved) = crate::util::path::make_absolute(&output) else { return Err(InvalidPath { path: output.clone(), } .into()); }; - absolute_path = current_path.join(output); + absolute_path = resolved; } absolute_path }; @@ -295,14 +295,14 @@ pub async fn write_address( let destination = { let mut absolute_path = Path::new(&output).to_path_buf(); if !absolute_path.is_absolute() { - let Ok(current_path) = std::env::current_dir() else { + let Ok(resolved) = crate::util::path::make_absolute(&output) else { return Err(InvalidPath { path: output.clone(), } .into()); }; - absolute_path = current_path.join(output); + absolute_path = resolved; } absolute_path }; diff --git a/lore-revision/src/util/path.rs b/lore-revision/src/util/path.rs index 477156e0..321cdc8d 100644 --- a/lore-revision/src/util/path.rs +++ b/lore-revision/src/util/path.rs @@ -16,19 +16,18 @@ pub enum PathError { InvalidPath, } -/// The directory the call in progress resolves relative paths against, when it -/// named one. A call arriving over IPC carries the directory of the process -/// that made it, because the service's own is unrelated to the caller's. -fn call_working_directory() -> Option { - let context = crate::runtime::try_execution_context()?; - let directory = context.globals().working_directory()?; - Some(PathBuf::from(directory)) -} - /// Resolves `path` against the working directory of the call in progress, or /// against this process's own when the call did not name one. +/// +/// A call arriving over IPC carries the directory of the process that made it, +/// because the service's own is unrelated to the caller's. pub fn make_absolute(path: impl AsRef) -> Result { - make_absolute_from(path, call_working_directory()) + let context = crate::runtime::try_execution_context(); + let base = context + .as_ref() + .and_then(|context| context.globals().working_directory()) + .map(Path::new); + make_absolute_from(path, base) } /// [`make_absolute`] with the base directory supplied by the caller, for the @@ -36,7 +35,7 @@ pub fn make_absolute(path: impl AsRef) -> Result { /// cannot look it up. pub fn make_absolute_from( path: impl AsRef, - base: Option, + base: Option<&Path>, ) -> Result { let path = path.as_ref(); let cleanpath = clean(path.to_owned()); @@ -46,13 +45,14 @@ pub fn make_absolute_from( if pathbuf.is_absolute() { return Ok(pathbuf); } - let base = match base { - Some(base) => base, - None => std::env::current_dir().emit_map_err(PathError::internal( - "failed to get current working directory", - ))?, - }; - Ok(base.join(pathbuf)) + match base { + Some(base) => Ok(base.join(pathbuf)), + None => Ok(std::env::current_dir() + .emit_map_err(PathError::internal( + "failed to get current working directory", + ))? + .join(pathbuf)), + } } /// Returns `true` when `candidate` resolves to a location inside diff --git a/lore/src/call.rs b/lore/src/call.rs index 8106e8a7..8e6f8246 100644 --- a/lore/src/call.rs +++ b/lore/src/call.rs @@ -1,5 +1,6 @@ // SPDX-FileCopyrightText: 2026 Epic Games, Inc. // SPDX-License-Identifier: MIT +use std::path::Path; use std::path::PathBuf; use std::sync::Arc; use std::time::Instant; @@ -221,10 +222,10 @@ async fn prepare_repository_call( mut globals: LoreGlobalArgs, callback: LoreEventCallback, ) -> Result<(PathBuf, Arc), i32> { - let working_directory = globals.working_directory().map(PathBuf::from); - let repository_path = if let Ok(path) = - util::path::make_absolute_from(globals.repository_path.as_str(), working_directory) - { + let repository_path = if let Ok(path) = util::path::make_absolute_from( + globals.repository_path.as_str(), + globals.working_directory().map(Path::new), + ) { globals.repository_path = path.display().to_string().into(); path } else { From c8e063dfef75ce003831a47106eb351afcd5e4e9 Mon Sep 17 00:00:00 2001 From: Mattias Jansson Date: Mon, 20 Jul 2026 15:08:16 +0200 Subject: [PATCH 3/6] Add a smoke test for working directory handling # Summary Nothing covered relative paths crossing to the service. The test harness passes `--repository` as an absolute path and runs commands from the repository root, so no existing test sends a relative path, and the service's own working directory never mattered. The bug this covers was therefore invisible to the suite. ## Changes - `scripts/test/conftest.py`: `lore_service_in_directory` starts the service in a chosen directory, which `background_lore_service` cannot do because it inherits the directory pytest runs in. Both are needed: a test that wants the service's directory to differ from the caller's has to place it deliberately. - `scripts/test/test_service.py`: start the service in one directory, clone to a relative path from another, then stage a relative path from inside the clone. Asserts the clone lands under the caller's directory and, just as importantly, that nothing appears under the service's, so the test fails rather than passes by accident if resolution silently changes again. # Test Plan Not run: the clone step needs a server, which is not available here. Checked statically that the module parses, that every fixture it names is defined, and that the CLI invocations match the argument shapes the existing wrappers build (`clone ` positionally, `stage `). The resolution the test asserts was verified by hand on macOS while making the change, with the service started in one directory and commands run from another, through both the explicit and the context resolution paths. Signed-off-by: Mattias Jansson --- scripts/test/conftest.py | 25 +++++++++++++ scripts/test/test_service.py | 71 ++++++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+) diff --git a/scripts/test/conftest.py b/scripts/test/conftest.py index 4e8b7c6a..0f6aad81 100644 --- a/scripts/test/conftest.py +++ b/scripts/test/conftest.py @@ -204,6 +204,31 @@ def _wait_for_service_ready(lore_executable_path, service_process, attempts=30): pytest.fail("Timed out waiting for Lore background service to accept connections") +@pytest.fixture(scope="function") +def lore_service_in_directory(lore_executable_path): + """Starts the service in a chosen directory, for tests that need the + service's own working directory to differ from the caller's.""" + processes = [] + + def start(directory): + service_process = subprocess.Popen( + [lore_executable_path, "service", "run"], cwd=str(directory) + ) + processes.append(service_process) + _wait_for_service_ready(lore_executable_path, service_process) + return service_process + + yield start + + for service_process in processes: + service_process.terminate() + try: + service_process.wait(timeout=10) + except subprocess.TimeoutExpired: + logger.warning("Lore service did not exit on terminate, killing it") + service_process.kill() + + @pytest.fixture(scope="function") def background_lore_service(lore_executable_path): command_args = [lore_executable_path, "service", "run"] diff --git a/scripts/test/test_service.py b/scripts/test/test_service.py index 9e34ef06..2012f9ee 100644 --- a/scripts/test/test_service.py +++ b/scripts/test/test_service.py @@ -3,6 +3,7 @@ import logging import os import platform +import subprocess import pytest @@ -19,6 +20,7 @@ def service_supported(): @pytest.mark.smoke +@pytest.mark.xdist_group("lore_service") @pytest.mark.skip(reason="Unknown issue specifically running in CI for OSS") @pytest.mark.skipif( not service_supported(), reason="Service not supported on " + platform.system() @@ -29,6 +31,7 @@ def test_service_down(new_lore_repo): @pytest.mark.smoke +@pytest.mark.xdist_group("lore_service") @pytest.mark.skipif( not service_supported(), reason="Service not supported on " + platform.system() ) @@ -48,3 +51,71 @@ def test_service_call(new_lore_repo, background_lore_service): assert "A " + file_name in map( lambda line: line.strip(" "), status_output.splitlines() ) + + +@pytest.mark.smoke +@pytest.mark.xdist_group("lore_service") +@pytest.mark.skipif( + not service_supported(), reason="Service not supported on " + platform.system() +) +def test_service_resolves_relative_paths_against_caller( + new_lore_repo, lore_executable_path, lore_service_in_directory, tmp_path +): + """Relative paths belong to the directory the command was run in. + + The service resolves them, and its own working directory is unrelated to + the caller's, so a service started elsewhere must not pull them towards + itself. Every other service test passes an absolute repository path, which + cannot catch this. + """ + repo: Lore = new_lore_repo() + with repo.open_file("seed.txt", "w+") as seed_file: + seed_file.write("seed\n") + repo.stage(scan=True, offline=True) + repo.commit("Seed", offline=True) + repo.push() + + service_directory = tmp_path / "service_elsewhere" + caller_directory = tmp_path / "caller" + service_directory.mkdir() + caller_directory.mkdir() + lore_service_in_directory(service_directory) + + environment = os.environ.copy() + environment.update(LORE_SERVICE_ENVIRONMENT) + + def run_in(directory, args): + result = subprocess.run( + [lore_executable_path, *args], + capture_output=True, + text=True, + cwd=str(directory), + env=environment, + ) + assert result.returncode == 0, ( + f"{args} failed in {directory}: {result.stdout}{result.stderr}" + ) + return result.stdout + result.stderr + + clone_name = "relative_clone" + run_in(caller_directory, ["clone", repo.remote_path, clone_name]) + + clone_path = caller_directory / clone_name + assert (clone_path / ".lore").is_dir(), ( + f"Clone must land under the caller's directory, not the service's. " + f"{caller_directory} contains {list(caller_directory.iterdir())}" + ) + assert not (service_directory / clone_name).exists(), ( + f"Clone must not land under the service's directory. " + f"{service_directory} contains {list(service_directory.iterdir())}" + ) + + file_name = "added.uasset" + (clone_path / file_name).write_bytes(os.urandom(30)) + + run_in(clone_path, ["stage", file_name]) + + status_output = run_in(clone_path, ["status"]) + assert "A " + file_name in map( + lambda line: line.strip(" "), status_output.splitlines() + ), f"Staged file should show as added: {status_output}" From babfe639871a0ade2572ac047fb3713648f8ba61 Mon Sep 17 00:00:00 2001 From: Mattias Jansson Date: Mon, 20 Jul 2026 16:20:51 +0200 Subject: [PATCH 4/6] Resolve user supplied paths against the caller's directory # Summary The smoke test added alongside this found a second way a relative path reached the wrong directory. `RelativePath::new_from_user_path`, which validates every user supplied path against the repository, resolved with `std::path::absolute`. That reads the process working directory just as `current_dir` does, so a path argument sent to the service was resolved against the service's directory. Staging a relative path through the service therefore staged nothing, and reported success while doing so, because the resolved path fell outside the repository and matched no file. The earlier audit missed this: it searched for `current_dir` and this is a different function reaching the same process state. ## Changes - `lore-revision/src/util/path.rs`: resolve both the user path and the repository path through `make_absolute`, which uses the directory the call carries. # Test Plan - `scripts/test/test_service.py`: 2 passed, 1 skipped, against a running server. The new test fails before this change at the staging step and passes after, which is how the bug was found. - `cargo clippy --all-targets -- -D warnings --no-deps`, `cargo +nightly fmt --all --check`, `cargo test -p lore-revision --lib` (159), `cargo test -p lore --lib` (90): all clean. `std::path::absolute` has no other uses in the library. The two remaining ones are in the CLI, which runs in the directory the user is actually in. Signed-off-by: Mattias Jansson --- lore-revision/src/util/path.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lore-revision/src/util/path.rs b/lore-revision/src/util/path.rs index 321cdc8d..a181963e 100644 --- a/lore-revision/src/util/path.rs +++ b/lore-revision/src/util/path.rs @@ -673,15 +673,13 @@ impl RelativePathBuf { let mut absolute_path = Path::new(user_path).to_path_buf(); if !absolute_path.is_absolute() { - absolute_path = std::path::absolute(absolute_path) - .emit_map_err(PathError::internal("failed to resolve absolute path"))?; + absolute_path = make_absolute(absolute_path.to_string_lossy())?; } let absolute_path = clean(absolute_path.display().to_string()); let mut repository_path = Path::new(repository_path).to_path_buf(); if !repository_path.is_absolute() { - repository_path = std::path::absolute(repository_path) - .emit_map_err(PathError::internal("failed to resolve absolute path"))?; + repository_path = make_absolute(repository_path.to_string_lossy())?; } let repository_path = clean(repository_path.display().to_string()).to_lowercase(); From 52fe4fc5c853e6839833084e9953230a5c61be29 Mon Sep 17 00:00:00 2001 From: Mattias Jansson Date: Wed, 22 Jul 2026 07:51:48 +0200 Subject: [PATCH 5/6] Set the caller's working directory in the library, not the CLI # Summary The working directory a service call resolves relative paths against was filled in by the CLI. A C API caller that routes through the service did not set it, so its relative paths resolved against the service's directory rather than its own. Move the defaulting into the library, at the point a call is sent to the service, so every caller gets it. A caller that sets the field keeps its value, so an installed tool running from a fixed directory can still direct relative paths elsewhere. ## Changes - `lore/src/remote/call.rs`: `service_call` fills `working_directory` with the process directory when the caller left it unset, before the globals are sent over IPC. - `lore-client/src/cli/cli.rs`: drop the CLI's own assignment, now redundant. A command that runs locally still resolves against the process directory through `make_absolute`'s fallback, so nothing changes for the non-service path. # Test Plan - `uv run pytest scripts/test/test_service.py`: 2 passed, 1 skipped. The relative-path test drives the real binary through the service with the CLI assignment removed, so its passing shows the library now supplies the directory. - `cargo clippy --all-targets -- -D warnings --no-deps`, `cargo +nightly fmt --all --check`, `cargo test -p lore --lib` (92): all clean. Signed-off-by: Mattias Jansson --- lore-client/src/cli/cli.rs | 4 ---- lore/src/remote/call.rs | 19 ++++++++++++++++++- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/lore-client/src/cli/cli.rs b/lore-client/src/cli/cli.rs index 245aa68e..076078cc 100644 --- a/lore-client/src/cli/cli.rs +++ b/lore-client/src/cli/cli.rs @@ -349,10 +349,6 @@ pub enum LoreCliError { pub fn lore_globals_from_args(cli: &LoreCli) -> LoreGlobalArgs { let mut args = LoreGlobalArgs { repository_path: get_repository_path(cli.repository.clone()), - working_directory: std::env::current_dir() - .map(|path| path.display().to_string()) - .unwrap_or_default() - .into(), force: cli.force.into(), dry_run: cli.dry_run.into(), diff --git a/lore/src/remote/call.rs b/lore/src/remote/call.rs index 224cc298..e71be0a0 100644 --- a/lore/src/remote/call.rs +++ b/lore/src/remote/call.rs @@ -34,11 +34,28 @@ impl EventError for ServiceCallError { } } +/// Records the directory the service resolves this call's relative paths +/// against, when the caller left it unset. The service runs in a directory +/// unrelated to the caller's, so without this a relative path would resolve +/// there rather than where the caller ran. A caller that set the field, such as +/// an installed tool that runs from a fixed directory but wants relative paths +/// resolved elsewhere, keeps its value. +#[allow(clippy::disallowed_methods)] +fn fill_working_directory(globals: &mut LoreGlobalArgs) { + if globals.working_directory().is_some() { + return; + } + if let Ok(directory) = std::env::current_dir() { + globals.working_directory = directory.display().to_string().into(); + } +} + pub async fn service_call( - globals: LoreGlobalArgs, + mut globals: LoreGlobalArgs, args: ArgsType, callback: LoreEventCallback, ) -> i32 { + fill_working_directory(&mut globals); let mut event_dispatcher = EventDispatcher::new(callback); service_call_impl(&mut event_dispatcher, globals, args) From ae21456a437f155c978a41fd5e576007574a5a3c Mon Sep 17 00:00:00 2001 From: Mattias Jansson Date: Wed, 22 Jul 2026 08:00:42 +0200 Subject: [PATCH 6/6] Drive the working directory test through the Lore test helpers # Summary The working directory test built its own subprocess calls with a hand-rolled environment. `Lore.run` already takes a `cwd` argument and merges the repository's `environment_vars`, and it also isolates the global config and auth token store per test, which the hand-rolled calls skipped. Route the test through those helpers instead. ## Changes - `scripts/test/test_service.py`: create the source repository with `environment_vars=LORE_SERVICE_ENVIRONMENT`, so every command routes through the service, and use `cwd=` on the clone and the clone's own `stage`/`status` to place each command's directory. Drops the `run_in` closure and its manual environment. - `scripts/test/conftest.py`: `lore_service_in_directory` now starts the service with the test's `LORE_GLOBAL_PATH`, so a shared store the service creates during the clone lands where the client looks for it. # Test Plan - `uv run pytest scripts/test/test_service.py`: 2 passed, 1 skipped, three times, stable. The relative-path test still fails at the staging step when the library resolves against the service's directory and passes once it resolves against the caller's. - `uv run ruff check`: clean. Signed-off-by: Mattias Jansson --- scripts/test/conftest.py | 10 ++++-- scripts/test/test_service.py | 60 +++++++++++++++++++----------------- 2 files changed, 39 insertions(+), 31 deletions(-) diff --git a/scripts/test/conftest.py b/scripts/test/conftest.py index 0f6aad81..f5ff3797 100644 --- a/scripts/test/conftest.py +++ b/scripts/test/conftest.py @@ -205,14 +205,18 @@ def _wait_for_service_ready(lore_executable_path, service_process, attempts=30): @pytest.fixture(scope="function") -def lore_service_in_directory(lore_executable_path): +def lore_service_in_directory(lore_executable_path, global_dir_name): """Starts the service in a chosen directory, for tests that need the - service's own working directory to differ from the caller's.""" + service's own working directory to differ from the caller's. The service + shares the test's isolated global config so shared stores it creates land + where the client looks for them.""" processes = [] def start(directory): + env = os.environ.copy() + env["LORE_GLOBAL_PATH"] = global_dir_name service_process = subprocess.Popen( - [lore_executable_path, "service", "run"], cwd=str(directory) + [lore_executable_path, "service", "run"], cwd=str(directory), env=env ) processes.append(service_process) _wait_for_service_ready(lore_executable_path, service_process) diff --git a/scripts/test/test_service.py b/scripts/test/test_service.py index 2012f9ee..a09d6c7d 100644 --- a/scripts/test/test_service.py +++ b/scripts/test/test_service.py @@ -3,7 +3,6 @@ import logging import os import platform -import subprocess import pytest @@ -59,7 +58,7 @@ def test_service_call(new_lore_repo, background_lore_service): not service_supported(), reason="Service not supported on " + platform.system() ) def test_service_resolves_relative_paths_against_caller( - new_lore_repo, lore_executable_path, lore_service_in_directory, tmp_path + new_lore_repo, lore_service_in_directory, tmp_path ): """Relative paths belong to the directory the command was run in. @@ -68,37 +67,32 @@ def test_service_resolves_relative_paths_against_caller( itself. Every other service test passes an absolute repository path, which cannot catch this. """ - repo: Lore = new_lore_repo() - with repo.open_file("seed.txt", "w+") as seed_file: - seed_file.write("seed\n") - repo.stage(scan=True, offline=True) - repo.commit("Seed", offline=True) - repo.push() - + # Start the service in a directory unrelated to where the commands run, so + # that a relative path resolved there rather than at the caller would show. service_directory = tmp_path / "service_elsewhere" caller_directory = tmp_path / "caller" service_directory.mkdir() caller_directory.mkdir() lore_service_in_directory(service_directory) - environment = os.environ.copy() - environment.update(LORE_SERVICE_ENVIRONMENT) - - def run_in(directory, args): - result = subprocess.run( - [lore_executable_path, *args], - capture_output=True, - text=True, - cwd=str(directory), - env=environment, - ) - assert result.returncode == 0, ( - f"{args} failed in {directory}: {result.stdout}{result.stderr}" - ) - return result.stdout + result.stderr + # Seed a remote to clone from. Routed through the service like the rest, + # but against the repository's own absolute path, so unaffected by the + # service's directory. + source: Lore = new_lore_repo(environment_vars=LORE_SERVICE_ENVIRONMENT.copy()) + with source.open_file("seed.txt", "w+") as seed_file: + seed_file.write("seed\n") + source.stage(scan=True, offline=True) + source.commit("Seed", offline=True) + source.push() + # Clone to a relative path from the caller's directory. It must land there, + # not under the service's directory. clone_name = "relative_clone" - run_in(caller_directory, ["clone", repo.remote_path, clone_name]) + source.run( + ["repository", "clone", source.remote_path, clone_name], + cwd=str(caller_directory), + use_os_dir=True, + ) clone_path = caller_directory / clone_name assert (clone_path / ".lore").is_dir(), ( @@ -110,12 +104,22 @@ def run_in(directory, args): f"{service_directory} contains {list(service_directory.iterdir())}" ) + # Stage a relative path from inside the clone. + clone = Lore( + lore_executable_path=source.lore_executable_path, + path=str(clone_path), + name=clone_name, + global_dir=source.global_dir, + environment_vars=LORE_SERVICE_ENVIRONMENT.copy(), + remote_url=source.remote, + remote_path=source.remote_path, + create_repo=False, + ) file_name = "added.uasset" (clone_path / file_name).write_bytes(os.urandom(30)) + clone.stage(file_name, relative_paths=True) - run_in(clone_path, ["stage", file_name]) - - status_output = run_in(clone_path, ["status"]) + status_output = clone.status() assert "A " + file_name in map( lambda line: line.strip(" "), status_output.splitlines() ), f"Staged file should show as added: {status_output}"