From 6cfc18baa10d45a4b6c7673bfcfe7dd256b10a48 Mon Sep 17 00:00:00 2001 From: Mattias Jansson Date: Mon, 20 Jul 2026 12:09:01 +0200 Subject: [PATCH 01/20] Implement service start and stop `lore service start` and `lore service stop` were stubs returning failure on every platform. Their original TODOs described a per-repository model: a sentinel file in `.lore/` marking a repository as service-controlled, and a registry of serviced repositories inside the service process. This instead makes using the service a global setting, in the same shape as `use_shared_store_automatically`. When it is set, the library routes every call through the service and starts one if none is running, so there is nothing to register and no per-repository state to track. `start` and `stop` become utilities to control that process explicitly. No sentinel file is written, and `repository::SERVICE` stays unused. There is still no system service integration; the process is an ordinary detached child, not a systemd unit or a Windows Service. - `lore-revision/src/global.rs`: new `use_service_automatically` field on `GlobalConfig`, with an accessor next to the shared-store one. - `lore/src/call_delegation.rs`: `dispatch_call` consults that setting rather than only the `LORE_USE_SERVICE` environment variable. Because every public entry point calls it and reading the setting parses the global TOML config, the value is cached for the life of the process and invalidated when written. `LORE_USE_SERVICE` remains as a per-invocation override; it now also understands `0` and `false` as "off", where previously any non-empty value meant "on". A call already executing inside the service runs locally, so it cannot dispatch back into the service it is running in. - `lore/src/remote/process.rs` (new): liveness check, detached spawn of `lore service run` (`setsid` on unix, `DETACHED_PROCESS` on Windows), and waits for the service to start and stop. Automatic start-up refuses to run anything not named `lore`, so an embedder does not have its own executable relaunched with `service run` arguments. It also holds the service's shutdown flag. Publishing the flag is what marks a process as the service, which is how `stop` distinguishes delivering a request from receiving one. - `lore/src/remote/call.rs`: `service_call` starts the service when none is listening, so a failure to start is reported through the existing event path rather than surfacing as a bare connection error. Adds `service_send_no_reply` for requests whose effect is that the service exits, where waiting for a reply would race the service's own shutdown. - `lore/src/service.rs`: implements all three handlers. `start` ensures a service is running; `stop` sends the request and waits for the socket to be released, or trips the accept loop directly when it is the service receiving that request; `set_use_automatically` writes the global config. None of the three route through `dispatch_call`, which would otherwise start the service in order to ask it to stop, or to be told it should no longer be used. Starting when a service is already running, and stopping when none is, are no-ops rather than errors. - `lore-client/src/cli/commands/service/run.rs`: the service publishes its shutdown flag at start-up, and now waits for the accept loop however it was stopped. Waiting on the termination signal alone left the process alive after a stop delivered over IPC: the accept loop ended and the socket was unlinked, but nothing woke `service_main`. - `LoreServiceStopArgs::all` is removed. It meant "stop servicing all repositories", which has no meaning without per-repository servicing. The `stop` subcommand takes no arguments, and `set-use-automatically` is added alongside it. - FFI: `lore_service_set_use_automatically` and its async form, with the cbindgen rename and the regenerated header. - Docs: the global `config.toml` was not documented at all, so `lore-cli-config.md` gains a section for it covering all three of its fields, not just the new one. `lore-cli-commands.md` is regenerated; that picks up pre-existing drift in the global flags as well as the service subcommands. - `cargo clippy --all-targets -- -D warnings --no-deps`, `cargo +nightly fmt --all --check`, and `cargo test -p lore --lib` (90 passed): all clean. - Verified end to end on macOS against an isolated `LORE_GLOBAL_PATH`: - With the setting unset, a command starts no service. - With it set, a command starts one and is served by it. - `start` when a service is running, and `stop` when none is, both exit 0. - `stop` terminates the process and releases the socket; SIGTERM still does the same. - `LORE_USE_SERVICE=0` overrides the setting off. - Disabling the setting does not start a service. - Routing was confirmed rather than assumed: with the setting on and automatic start-up refused (the executable renamed so it is not `lore`), a command fails at the service boundary, while the same binary with the override off succeeds locally. Comparing output alone does not distinguish the two paths. Not verified on Windows: no `x86_64-pc-windows-msvc` target is installed locally, and the detached-spawn flags there have not been exercised. The Python tests are likewise unverified, since they need a running server; they also assume sequential execution, as the service socket is per user rather than per test. Signed-off-by: Mattias Jansson --- docs/reference/README.md | 2 +- docs/reference/lore-cli-commands.md | 26 ++- docs/reference/lore-cli-config.md | 33 +++- lore-capi/lore.h | 56 ++++++- lore-client/src/cli/commands/service.rs | 50 ++++-- lore-client/src/cli/commands/service/run.rs | 44 +++-- lore-revision/src/global.rs | 4 + lore/cbindgen.toml | 1 + lore/src/call_delegation.rs | 64 +++++++- lore/src/interface.rs | 65 ++++++++ lore/src/remote.rs | 21 +++ lore/src/remote/call.rs | 38 +++++ lore/src/remote/command.rs | 1 + lore/src/remote/network.rs | 4 + lore/src/remote/network/unix.rs | 6 +- lore/src/remote/network/windows.rs | 4 +- lore/src/remote/process.rs | 168 ++++++++++++++++++++ lore/src/service.rs | 156 ++++++++++++++---- scripts/test/lore.py | 12 +- scripts/test/test_service.py | 94 ++++++++++- 20 files changed, 753 insertions(+), 96 deletions(-) create mode 100644 lore/src/remote/process.rs diff --git a/docs/reference/README.md b/docs/reference/README.md index 2765bf90..9b670ba4 100644 --- a/docs/reference/README.md +++ b/docs/reference/README.md @@ -9,7 +9,7 @@ Reference is the technical description of the machinery and how to operate it. A ## Reference pages - [Lore CLI command reference](lore-cli-commands.md) — every `lore` command, subcommand, argument, and flag, generated from `lore --markdown-help`. -- [Lore CLI configuration reference](lore-cli-config.md) — every field in the per-repository `config.toml` and user-level `cli.toml`, with each field's type, default, and on-disk location. +- [Lore CLI configuration reference](lore-cli-config.md) — every field in the per-repository `config.toml`, the user-level global `config.toml`, and the user-level `cli.toml`, with each field's type, default, and on-disk location. - [Lore Server configuration reference](lore-server-config.md) — every `loreserver` CLI flag, config-file layer, and settings field, including the AWS, DynamoDB, Consul, and hook plugin backends. ## Suggested starting points diff --git a/docs/reference/lore-cli-commands.md b/docs/reference/lore-cli-commands.md index d0e74551..44176dbd 100644 --- a/docs/reference/lore-cli-commands.md +++ b/docs/reference/lore-cli-commands.md @@ -161,6 +161,7 @@ This page is generated from `lore --markdown-help` (CLI `0.8.2-nightly+31`). Eve * [`lore service run`↴](#lore-service-run) * [`lore service start`↴](#lore-service-start) * [`lore service stop`↴](#lore-service-stop) +* [`lore service set-use-automatically`↴](#lore-service-set-use-automatically) * [`lore notification`↴](#lore-notification) * [`lore notification subscribe`↴](#lore-notification-subscribe) * [`lore completions`↴](#lore-completions) @@ -219,8 +220,9 @@ This page is generated from `lore --markdown-help` (CLI `0.8.2-nightly+31`). Eve * `--compress-limit ` — Set maximum number of parallel compress operations * `--search-limit ` — Set maximum number of revisions to search when matching or finding revisions * `--search-nearest` — Set to search for nearest match when matching revisions -* `--gc` — Set to run automatic garbage collection on local store in background +* `--no-gc` — Prevent automatic incremental garbage collection for this command; it otherwise runs in the background on writes. `lore repository gc` always runs a full pass regardless * `--sync-data` — Force sync data to storage media during flush +* `--cache` — Cache fragment payloads fetched from remote in the local store * `--non-interactive` — Disable interactive prompts (e.g., per-link commit messages) @@ -2598,8 +2600,9 @@ Manage the repository in a service process ###### **Subcommands:** * `run` — Run this process as the service -* `start` — Start service for a repository -* `stop` — Stop service for a repository +* `start` — Start the service process +* `stop` — Stop the service process +* `set-use-automatically` — Set whether to automatically use the service process @@ -2613,7 +2616,7 @@ Run this process as the service ## `lore service start` -Start service for a repository +Start the service process **Usage:** `lore service start` @@ -2621,13 +2624,21 @@ Start service for a repository ## `lore service stop` -Stop service for a repository +Stop the service process -**Usage:** `lore service stop [all]` +**Usage:** `lore service stop` + + + +## `lore service set-use-automatically` + +Set whether to automatically use the service process + +**Usage:** `lore service set-use-automatically ` ###### **Arguments:** -* `` — Flag to stop servicing all repositories +* `` — Automatically run Lore commands through the service process Possible values: `true`, `false` @@ -2731,3 +2742,4 @@ Manage the shared store This document was generated automatically by clap-markdown. + diff --git a/docs/reference/lore-cli-config.md b/docs/reference/lore-cli-config.md index beee891f..c2d8682d 100644 --- a/docs/reference/lore-cli-config.md +++ b/docs/reference/lore-cli-config.md @@ -4,10 +4,11 @@ ```text /.lore/config.toml # per-repository client settings (created on init/clone) +~/.config/lore/config.toml # user-level global settings (OS user config dir; Linux shown) ~/.config/lore/cli.toml # user-level CLI settings (OS user config dir; Linux shown) ``` -This page documents the **Lore CLI** client configuration: the per-repository `config.toml` and the user-level `cli.toml` that the `lore` binary reads. These are distinct from the Lore Server daemon's configuration — for server stores, endpoints, topology, and plugin backends, see the [Lore Server configuration reference](lore-server-config.md). The fields below are written by `lore repository create` and `lore clone`, or you edit them by hand; you don't need to read the source to look one up. +This page documents the **Lore CLI** client configuration: the per-repository `config.toml`, the user-level global `config.toml`, and the user-level `cli.toml` that the `lore` binary reads. These are distinct from the Lore Server daemon's configuration — for server stores, endpoints, topology, and plugin backends, see the [Lore Server configuration reference](lore-server-config.md). The fields below are written by `lore repository create` and `lore clone`, or you edit them by hand; you don't need to read the source to look one up. ## Per-repository `config.toml` @@ -87,6 +88,36 @@ A config that uses the legacy names still loads. New configs use the current nam Lore normally writes this table for you when you clone with `--use-shared-store`. For how shared stores work and how to set one up, see [Step 6 of the Quickstart](../tutorials/quickstart.md#step-6-set-up-a-shared-store-and-clone-a-second-working-tree); for the `lore clone` and `lore shared-store` flags, see the [Lore CLI command reference](lore-cli-commands.md). +## User-level global `config.toml` + +### Location + +The global `config.toml` holds settings that apply to every repository, rather than to one. It shares the OS user config directory with `cli.toml`, so on a typical Linux setup it is `~/.config/lore/config.toml`; see the table under `cli.toml` below for the other platforms. Setting `LORE_GLOBAL_PATH` moves the whole directory, which is how the test suite isolates it. + +The file is optional. When it is absent, every setting below takes its default. + +### Fields + +| Field | Type | Default | Description | +| --- | --- | --- | --- | +| `use_shared_store_automatically` | bool | `false` | Whether `lore repository create` and `lore clone` configure a shared store without being asked. Read only when a repository is created or cloned; the result is written into that repository's own config. | +| `use_service_automatically` | bool | `false` | Whether Lore runs every command in the background service process. See below. | +| `default_shared_stores` | table | empty | Per-remote default shared store paths, keyed by remote URL. | + +### Running commands through the service + +With `use_service_automatically` enabled, Lore sends each command to a background service process over a local socket instead of executing it in the CLI process, and starts that service automatically if it is not already running. Set it with: + +```bash +lore service set-use-automatically true +``` + +`lore service start` and `lore service stop` control the process explicitly. Starting when a service is already running, and stopping when none is, are both no-ops rather than errors. + +The `LORE_USE_SERVICE` environment variable overrides the setting for a single invocation: any value other than `0` or `false` forces the service on, and `0`, `false`, or an empty value forces it off. + +Automatic start-up only applies to the `lore` CLI. When Lore is embedded as a library the running executable is the host application, which Lore will not relaunch as a service; start the service separately in that case. + ## User-level `cli.toml` ### Location diff --git a/lore-capi/lore.h b/lore-capi/lore.h index f04f7eb6..bcbc104f 100644 --- a/lore-capi/lore.h +++ b/lore-capi/lore.h @@ -4940,17 +4940,22 @@ typedef struct lore_storage_upload_args_t { struct lore_storage_upload_item_array_t items; } lore_storage_upload_args_t; -// Arguments for starting the Lore service process for the current repository (no parameters). +// Arguments for starting the Lore service process (no parameters). typedef struct lore_service_start_args_t { int _unused; } lore_service_start_args_t; -// Arguments for stopping the Lore service process for the current or all repositories. +// Arguments for stopping the Lore service process (no parameters). typedef struct lore_service_stop_args_t { - // Stop all repositories rather than just the current one - uint8_t all; + int _unused; } lore_service_stop_args_t; +// Arguments for setting whether Lore automatically routes calls through the service process. +typedef struct lore_service_set_use_automatically_args_t { + // Automatically use the service process + uint8_t enabled; +} lore_service_set_use_automatically_args_t; + // Arguments for subscribing to repository notifications (no parameters). typedef struct lore_notification_subscribe_args_t { int _unused; @@ -10472,6 +10477,49 @@ void lore_service_stop_async(const struct lore_global_args_t *globals, const struct lore_service_stop_args_t *args, struct lore_event_callback_config_t callback); +// Set whether Lore automatically routes calls through the background service. +// +// When enabled, every Lore call is executed by the service process, which is +// started automatically if it is not already running. +// +// # Events +// +// Events are delivered via the callback as `lore_event_t`. Use the `tag` field to identify the event type. +// +// ## Standard Events +// +// These events are emitted by all interface functions: +// +// | Tag | Data Type | Description | +// |-----|-----------|-------------| +// | `LORE_EVENT_LOG` | `lore_log_event_data_t` | Diagnostic messages throughout execution | +// | `LORE_EVENT_ERROR` | `lore_error_event_data_t` | Emitted for a non-fatal error during the operation | +// | `LORE_EVENT_COMPLETE` | `lore_complete_event_data_t` | Always emitted at the end; `status` is `0` on success or the error code on failure | +// | `LORE_EVENT_END` | `lore_end_event_data_t` | Always emitted after `COMPLETE` to signal callback termination | +int32_t lore_service_set_use_automatically(const struct lore_global_args_t *globals, + const struct lore_service_set_use_automatically_args_t *args, + struct lore_event_callback_config_t callback); + +// Asynchronous version of `lore_service_set_use_automatically`. +// +// # Events +// +// Events are delivered via the callback as `lore_event_t`. Use the `tag` field to identify the event type. +// +// ## Standard Events +// +// These events are emitted by all interface functions: +// +// | Tag | Data Type | Description | +// |-----|-----------|-------------| +// | `LORE_EVENT_LOG` | `lore_log_event_data_t` | Diagnostic messages throughout execution | +// | `LORE_EVENT_ERROR` | `lore_error_event_data_t` | Emitted for a non-fatal error during the operation | +// | `LORE_EVENT_COMPLETE` | `lore_complete_event_data_t` | Always emitted at the end; `status` is `0` on success or the error code on failure | +// | `LORE_EVENT_END` | `lore_end_event_data_t` | Always emitted after `COMPLETE` to signal callback termination | +void lore_service_set_use_automatically_async(const struct lore_global_args_t *globals, + const struct lore_service_set_use_automatically_args_t *args, + struct lore_event_callback_config_t callback); + // Subscribe to repository notifications. // // # Events diff --git a/lore-client/src/cli/commands/service.rs b/lore-client/src/cli/commands/service.rs index 2cdb4aa6..64c44287 100644 --- a/lore-client/src/cli/commands/service.rs +++ b/lore-client/src/cli/commands/service.rs @@ -6,6 +6,7 @@ use clap::Args; use clap::Subcommand; use lore::interface::LoreEvent; use lore::interface::LoreGlobalArgs; +use lore::interface::LoreServiceSetUseAutomaticallyArgs; use lore::interface::LoreServiceStartArgs; use lore::interface::LoreServiceStopArgs; use lore::runtime; @@ -32,10 +33,13 @@ pub struct ServiceRunArgs {} pub struct ServiceStartArgs {} #[derive(Args)] -pub struct ServiceStopArgs { - /// Flag to stop servicing all repositories - #[clap(value_name = "all")] - all: Option, +pub struct ServiceStopArgs {} + +#[derive(Args)] +pub struct ServiceSetUseAutomaticallyArgs { + /// Automatically run Lore commands through the service process + #[clap(value_name = "enabled", value_parser = clap::value_parser!(bool), action = clap::ArgAction::Set)] + enabled: bool, } #[derive(Subcommand)] @@ -43,11 +47,14 @@ pub enum ServiceCommands { ///Run this process as the service Run(ServiceRunArgs), - /// Start service for a repository + /// Start the service process Start(ServiceStartArgs), - /// Stop service for a repository + /// Stop the service process Stop(ServiceStopArgs), + + /// Set whether to automatically use the service process + SetUseAutomatically(ServiceSetUseAutomaticallyArgs), } fn handle_service_run(_globals: LoreGlobalArgs, _args: &ServiceRunArgs) -> u8 { @@ -81,10 +88,8 @@ fn handle_service_start(globals: LoreGlobalArgs, _args: &ServiceStartArgs) -> u8 return runtime().block_on(service::start(globals, start_args, callback)) as u8; } -fn handle_service_stop(globals: LoreGlobalArgs, args: &ServiceStopArgs) -> u8 { - let stop_args = LoreServiceStopArgs { - all: if args.all.unwrap_or_default() { 1 } else { 0 }, - }; +fn handle_service_stop(globals: LoreGlobalArgs, _args: &ServiceStopArgs) -> u8 { + let stop_args = LoreServiceStopArgs {}; let callback = output_formatter().unwrap_or(Some( (Box::new(move |event: &LoreEvent| match event { @@ -100,6 +105,28 @@ fn handle_service_stop(globals: LoreGlobalArgs, args: &ServiceStopArgs) -> u8 { return runtime().block_on(service::stop(globals, stop_args, callback)) as u8; } +fn handle_service_set_use_automatically( + globals: LoreGlobalArgs, + args: &ServiceSetUseAutomaticallyArgs, +) -> u8 { + let set_args = LoreServiceSetUseAutomaticallyArgs { + enabled: u8::from(args.enabled), + }; + + let callback = output_formatter().unwrap_or(Some( + (Box::new(move |event: &LoreEvent| match event { + LoreEvent::Complete(_) => {} + LoreEvent::Maintenance(data) => { + util::handle_maintenance_event(data); + } + _ => (), + }) as EventCallbackFn) + .with_defaults(), + )); + + return runtime().block_on(service::set_use_automatically(globals, set_args, callback)) as u8; +} + pub fn handle_service_commands(cmd: &ServiceCommands, globals: LoreGlobalArgs) -> u8 { match cmd { ServiceCommands::Run(args) => { @@ -111,5 +138,8 @@ pub fn handle_service_commands(cmd: &ServiceCommands, globals: LoreGlobalArgs) - ServiceCommands::Stop(args) => { return handle_service_stop(globals, args); } + ServiceCommands::SetUseAutomatically(args) => { + return handle_service_set_use_automatically(globals, args); + } } } diff --git a/lore-client/src/cli/commands/service/run.rs b/lore-client/src/cli/commands/service/run.rs index e6bc9656..64d9708d 100644 --- a/lore-client/src/cli/commands/service/run.rs +++ b/lore-client/src/cli/commands/service/run.rs @@ -4,7 +4,6 @@ use std::io::Write; use std::sync::Arc; use std::sync::atomic::AtomicBool; use std::sync::atomic::Ordering; -use std::time::Duration; use lore::interface::LoreEvent; use lore::remote::connection::ConnectionError; @@ -19,6 +18,7 @@ use lore::remote::message::write_v1_message; use lore::remote::network::UdsListener; use lore::remote::network::UdsStream; use lore::remote::network::uds_supported; +use lore::remote::process; use lore::runtime; use lore_error_set::prelude::*; use tokio::sync::mpsc; @@ -30,11 +30,6 @@ use crate::util::listen_for_termination; #[error_set] pub enum ServiceMainError {} -/// Bounds how long shutdown waits for the accept loop to unwind, so that a -/// wake-up connection that never lands cannot keep the process alive. Anything -/// 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 @@ -54,6 +49,13 @@ fn detached_working_directory() -> std::path::PathBuf { } } +/// Runs this process as the Lore service until it is stopped. +/// +/// Shutdown has two triggers, a termination signal and a `service stop` +/// delivered over IPC, and both end the same way: they set the shared flag and +/// connect to the socket so the blocked `accept` returns and the loop can +/// observe it. Waiting on the accept loop rather than on the signal is what +/// makes the IPC trigger end the process too. pub async fn service_main( listening_signal: Option>, ) -> Result<(), ServiceMainError> { @@ -79,6 +81,7 @@ pub async fn service_main( let shutting_down = Arc::new(AtomicBool::new(false)); let accept_shutting_down = Arc::clone(&shutting_down); + process::register_shutdown_flag(Arc::clone(&shutting_down)); let accept_task = runtime().spawn_blocking(move || { let mut connection_id = 0; @@ -106,27 +109,18 @@ pub async fn service_main( } }); - match listen_for_termination(None).await { - Ok(()) => { - println!("Shutting down Lore service"); - shutting_down.store(true, Ordering::SeqCst); - if let Err(error) = UdsStream::connect() { - eprintln!("Failed to wake the accept loop: {error}"); - } - if tokio::time::timeout(SHUTDOWN_TIMEOUT, accept_task) - .await - .is_err() - { - eprintln!("Timed out waiting for the accept loop to stop"); - } - } - Err(error) => { - // Without signal handling there is no graceful path, so keep - // serving until the process is killed. + runtime().spawn(async move { + if let Err(error) = listen_for_termination(None).await { eprintln!("Failed to listen for termination signals: {error}"); - let _ = accept_task.await; + return; } - } + println!("Shutting down Lore service"); + if let Err(error) = process::request_shutdown() { + eprintln!("Failed to stop the accept loop: {error}"); + } + }); + + let _ = accept_task.await; Ok(()) } diff --git a/lore-revision/src/global.rs b/lore-revision/src/global.rs index fa740af6..12bd0e82 100644 --- a/lore-revision/src/global.rs +++ b/lore-revision/src/global.rs @@ -74,6 +74,7 @@ pub struct GlobalConfig { default_shared_stores: BTreeMap, #[serde(alias = "use_global_store_automatically")] pub use_shared_store_automatically: Option, + pub use_service_automatically: Option, } impl GlobalConfig { @@ -115,6 +116,9 @@ impl GlobalConfig { pub fn use_shared_store_automatically(&self) -> bool { self.use_shared_store_automatically.unwrap_or(false) } + pub fn use_service_automatically(&self) -> bool { + self.use_service_automatically.unwrap_or(false) + } pub fn suggested_path_for_remote_url(remote_url: &str) -> Result { let data_dir = get_global_data_dir()?; let normalized = normalize_remote_url(remote_url); diff --git a/lore/cbindgen.toml b/lore/cbindgen.toml index a733adc5..a0edbf6b 100644 --- a/lore/cbindgen.toml +++ b/lore/cbindgen.toml @@ -414,6 +414,7 @@ include = [ "LoreSharedStoreSetUseAutomaticallyArgs" = "lore_shared_store_set_use_automatically_args_t" "LoreServiceStartArgs" = "lore_service_start_args_t" "LoreServiceStopArgs" = "lore_service_stop_args_t" +"LoreServiceSetUseAutomaticallyArgs" = "lore_service_set_use_automatically_args_t" "LoreStore" = "lore_store_t" "LoreStorageCloseArgs" = "lore_storage_close_args_t" "LoreStorageFlushArgs" = "lore_storage_flush_args_t" diff --git a/lore/src/call_delegation.rs b/lore/src/call_delegation.rs index 3d731234..a9f6c413 100644 --- a/lore/src/call_delegation.rs +++ b/lore/src/call_delegation.rs @@ -1,11 +1,64 @@ // SPDX-FileCopyrightText: 2026 Epic Games, Inc. // SPDX-License-Identifier: MIT +use lore_revision::global::GlobalConfig; use lore_revision::interface::LoreGlobalArgs; +use lore_revision::lore_debug; +use parking_lot::RwLock; use crate::args::InvokableLoreArgs; use crate::interface::LoreEventCallback; use crate::interface::LoreEventCallbackConfig; use crate::remote::call::service_call; +use crate::remote::process; + +const USE_SERVICE_VAR: &str = "LORE_USE_SERVICE"; + +/// Caches `use_service_automatically` for the life of the process. Every public +/// API entry point consults it, and reading it means parsing the global TOML +/// config, so it must not happen per call. +static USE_SERVICE: RwLock> = RwLock::new(None); + +/// Drops the cached setting so the next call rereads it. Called after the +/// setting is written, so a long-lived embedder does not keep using the old +/// value. +pub(crate) fn invalidate_use_service_cache() { + *USE_SERVICE.write() = None; +} + +/// `LORE_USE_SERVICE` overrides the stored setting when set, so that tests and +/// one-off invocations can route through the service without writing config. +/// An empty value, or `0`/`false`, means "do not use the service". +fn use_service_override() -> Option { + let value = std::env::var(USE_SERVICE_VAR).ok()?; + if value.is_empty() { + return Some(false); + } + Some(!value.eq_ignore_ascii_case("0") && !value.eq_ignore_ascii_case("false")) +} + +/// Whether calls should be routed through the service process. +/// +/// Test builds never read the stored setting, so that the suite runs against a +/// clean global config rather than whatever the developer has configured on +/// their own machine. `LORE_USE_SERVICE` is still honoured, so a test that wants +/// the service path can ask for it explicitly. +pub(crate) async fn use_service() -> bool { + if let Some(value) = use_service_override() { + return value; + } + if cfg!(test) { + return false; + } + if let Some(cached) = *USE_SERVICE.read() { + return cached; + } + let enabled = GlobalConfig::load() + .await + .map(|config| config.use_service_automatically()) + .unwrap_or(false); + *USE_SERVICE.write() = Some(enabled); + enabled +} pub(crate) fn run_synchronously< ArgsType: InvokableLoreArgs + Clone + Send + 'static, @@ -39,6 +92,9 @@ pub(crate) fn run_asynchronously< drop(lore_base::lore_spawn!(handler(globals, args, callback))); } +/// Runs a call either in the service process or locally, according to +/// [`use_service`]. A call already executing inside the service always runs +/// locally, so that it cannot dispatch back into the service it is running in. pub(crate) async fn dispatch_call< ArgsType: InvokableLoreArgs + Clone + Send + 'static, Handler: Fn(LoreGlobalArgs, ArgsType, LoreEventCallback) -> Fut, @@ -49,9 +105,11 @@ pub(crate) async fn dispatch_call< callback: LoreEventCallback, handler: Handler, ) -> i32 { - if let Ok(environment_value) = std::env::var("LORE_USE_SERVICE") - && !environment_value.is_empty() - { + if use_service().await && !process::running_as_service() { + lore_debug!( + "Using Lore service process for {}", + std::any::type_name::() + ); service_call(globals, args, callback).await } else { handler(globals, args, callback).await diff --git a/lore/src/interface.rs b/lore/src/interface.rs index e38a71c7..74e4ee7a 100644 --- a/lore/src/interface.rs +++ b/lore/src/interface.rs @@ -6804,6 +6804,71 @@ pub extern "C" fn lore_service_stop_async( run_asynchronously(globals, args, callback, crate::service::stop); } +pub type LoreServiceSetUseAutomaticallyArgs = crate::service::LoreServiceSetUseAutomaticallyArgs; + +/// Set whether Lore automatically routes calls through the background service. +/// +/// When enabled, every Lore call is executed by the service process, which is +/// started automatically if it is not already running. +/// +/// # Events +/// +/// Events are delivered via the callback as `lore_event_t`. Use the `tag` field to identify the event type. +/// +/// ## Standard Events +/// +/// These events are emitted by all interface functions: +/// +/// | Tag | Data Type | Description | +/// |-----|-----------|-------------| +/// | `LORE_EVENT_LOG` | `lore_log_event_data_t` | Diagnostic messages throughout execution | +/// | `LORE_EVENT_ERROR` | `lore_error_event_data_t` | Emitted for a non-fatal error during the operation | +/// | `LORE_EVENT_COMPLETE` | `lore_complete_event_data_t` | Always emitted at the end; `status` is `0` on success or the error code on failure | +/// | `LORE_EVENT_END` | `lore_end_event_data_t` | Always emitted after `COMPLETE` to signal callback termination | +#[unsafe(no_mangle)] +pub extern "C" fn lore_service_set_use_automatically( + globals: &LoreGlobalArgs, + args: &LoreServiceSetUseAutomaticallyArgs, + callback: LoreEventCallbackConfig, +) -> i32 { + run_synchronously( + globals, + args, + callback, + crate::service::set_use_automatically, + ) +} + +/// Asynchronous version of `lore_service_set_use_automatically`. +/// +/// # Events +/// +/// Events are delivered via the callback as `lore_event_t`. Use the `tag` field to identify the event type. +/// +/// ## Standard Events +/// +/// These events are emitted by all interface functions: +/// +/// | Tag | Data Type | Description | +/// |-----|-----------|-------------| +/// | `LORE_EVENT_LOG` | `lore_log_event_data_t` | Diagnostic messages throughout execution | +/// | `LORE_EVENT_ERROR` | `lore_error_event_data_t` | Emitted for a non-fatal error during the operation | +/// | `LORE_EVENT_COMPLETE` | `lore_complete_event_data_t` | Always emitted at the end; `status` is `0` on success or the error code on failure | +/// | `LORE_EVENT_END` | `lore_end_event_data_t` | Always emitted after `COMPLETE` to signal callback termination | +#[unsafe(no_mangle)] +pub extern "C" fn lore_service_set_use_automatically_async( + globals: &LoreGlobalArgs, + args: &LoreServiceSetUseAutomaticallyArgs, + callback: LoreEventCallbackConfig, +) { + run_asynchronously( + globals, + args, + callback, + crate::service::set_use_automatically, + ); +} + pub type LoreNotificationSubscribeArgs = crate::notification::LoreNotificationSubscribeArgs; /// Subscribe to repository notifications. diff --git a/lore/src/remote.rs b/lore/src/remote.rs index 92ac31ba..0f9b2a90 100644 --- a/lore/src/remote.rs +++ b/lore/src/remote.rs @@ -6,5 +6,26 @@ pub mod connection; pub mod message; pub mod network; +pub mod process; pub const LORE_SERVICE_SOCKET_NAME: &str = "lore_service"; + +#[cfg(test)] +static SOCKET_NAME_OVERRIDE: std::sync::OnceLock = std::sync::OnceLock::new(); + +/// Points the transport at a socket of its own for the duration of the test +/// binary, so that it neither collides with nor disturbs a service already +/// running on the machine. +#[cfg(test)] +pub(crate) fn set_service_socket_name_for_test(name: &str) { + let _ = SOCKET_NAME_OVERRIDE.set(name.to_owned()); +} + +/// The socket file name the service listens on. +pub(crate) fn service_socket_name() -> &'static str { + #[cfg(test)] + if let Some(name) = SOCKET_NAME_OVERRIDE.get() { + return name.as_str(); + } + LORE_SERVICE_SOCKET_NAME +} diff --git a/lore/src/remote/call.rs b/lore/src/remote/call.rs index e71be0a0..821ae74e 100644 --- a/lore/src/remote/call.rs +++ b/lore/src/remote/call.rs @@ -79,6 +79,10 @@ pub async fn service_call_impl( return Err(ServiceCallError::internal("OS doesn't support IPC")); } + crate::remote::process::ensure_running() + .await + .forward::("starting the Lore service")?; + let connection = lore_base::lore_spawn_blocking!(|| { let mut connection = UdsStream::connect().forward::("connecting to local socket")?; @@ -124,6 +128,40 @@ pub async fn service_call_impl( )) } +/// Sends one command to a service that is already running and returns as soon +/// as it is written, without waiting for a result. Used for requests whose +/// effect is that the service exits, where waiting for a reply would race the +/// service's own shutdown. Does not start a service. +pub async fn service_send_no_reply( + globals: LoreGlobalArgs, + args: ArgsType, +) -> Result<(), ServiceCallError> { + if !uds_supported() { + return Err(ServiceCallError::internal("OS doesn't support IPC")); + } + + lore_base::lore_spawn_blocking!(|| { + let mut connection = + UdsStream::connect().forward::("connecting to local socket")?; + + let message = MessageToServer { + globals, + command: args.to_command(), + }; + + let message_bytes = write_v1_message(message, SerializationType::Json) + .forward::("serializing message")?; + + connection + .writer() + .write_all(&message_bytes) + .internal("sending message")?; + Ok::<(), ServiceCallError>(()) + }) + .await + .internal("joining send task")? +} + pub fn handle_message( event_dispatcher: &mut EventDispatcher, message: MessageToClient, diff --git a/lore/src/remote/command.rs b/lore/src/remote/command.rs index cb40961f..8dcd1ef8 100644 --- a/lore/src/remote/command.rs +++ b/lore/src/remote/command.rs @@ -121,6 +121,7 @@ pub enum LoreCommand { RevisionSync(crate::revision::LoreRevisionSyncArgs), ServiceStart(crate::service::LoreServiceStartArgs), ServiceStop(crate::service::LoreServiceStopArgs), + ServiceSetUseAutomatically(crate::service::LoreServiceSetUseAutomaticallyArgs), NotificationSubscribe(crate::notification::LoreNotificationSubscribeArgs), NotificationUnsubscribe(crate::notification::LoreNotificationUnsubscribeArgs), SharedStoreCreate(crate::shared_store::LoreSharedStoreCreateArgs), diff --git a/lore/src/remote/network.rs b/lore/src/remote/network.rs index cb0f8e5f..c8bfdbfb 100644 --- a/lore/src/remote/network.rs +++ b/lore/src/remote/network.rs @@ -80,6 +80,10 @@ mod tests { } fn run_both() -> String { + crate::remote::set_service_socket_name_for_test(&format!( + "lore_service_test_{}", + std::process::id() + )); let (sender, receiver) = std::sync::mpsc::channel::<()>(); let service = std::thread::spawn(move || run_service(sender)); receiver.recv().unwrap(); diff --git a/lore/src/remote/network/unix.rs b/lore/src/remote/network/unix.rs index c467ea5b..89f35f5b 100644 --- a/lore/src/remote/network/unix.rs +++ b/lore/src/remote/network/unix.rs @@ -8,10 +8,10 @@ use std::path::PathBuf; use lore_error_set::prelude::*; -use crate::remote::LORE_SERVICE_SOCKET_NAME; use crate::remote::network::UdsAcceptError; use crate::remote::network::UdsConnectionError; use crate::remote::network::UdsListenerError; +use crate::remote::service_socket_name; pub fn uds_supported() -> bool { true @@ -37,7 +37,7 @@ fn uds_sock_dir() -> PathBuf { } fn uds_sock_path() -> PathBuf { - uds_sock_dir().join(LORE_SERVICE_SOCKET_NAME) + uds_sock_dir().join(service_socket_name()) } pub struct UdsListener { @@ -53,7 +53,7 @@ impl UdsListener { fs::set_permissions(&dir, fs::Permissions::from_mode(0o700)) .internal_with(|| format!("restricting socket directory {}", dir.display()))?; - let path = dir.join(LORE_SERVICE_SOCKET_NAME); + let path = dir.join(service_socket_name()); if path.exists() { if UnixStream::connect(&path).is_ok() { diff --git a/lore/src/remote/network/windows.rs b/lore/src/remote/network/windows.rs index 269a11b9..e00d097d 100644 --- a/lore/src/remote/network/windows.rs +++ b/lore/src/remote/network/windows.rs @@ -17,10 +17,10 @@ use windows_sys::Win32::Networking::WinSock::WSAGetLastError; use windows_sys::Win32::Storage::FileSystem::DeleteFileW; use windows_sys::Win32::Storage::FileSystem::GetTempPathW; -use crate::remote::LORE_SERVICE_SOCKET_NAME; use crate::remote::network::UdsAcceptError; use crate::remote::network::UdsConnectionError; use crate::remote::network::UdsListenerError; +use crate::remote::service_socket_name; const LISTENER_BACKLOG: i32 = 10; @@ -190,7 +190,7 @@ fn uds_sock_path() -> Vec { } // Append the file name, taking into account the null terminator. path.resize(path.len() - 1, 0); - path.extend(LORE_SERVICE_SOCKET_NAME.encode_utf16()); + path.extend(service_socket_name().encode_utf16()); // Reinsert the null terminator. path.push(0); path diff --git a/lore/src/remote/process.rs b/lore/src/remote/process.rs new file mode 100644 index 00000000..43c871e6 --- /dev/null +++ b/lore/src/remote/process.rs @@ -0,0 +1,168 @@ +// SPDX-FileCopyrightText: 2026 Epic Games, Inc. +// SPDX-License-Identifier: MIT +use std::process::Command; +use std::process::Stdio; +use std::sync::Arc; +use std::sync::OnceLock; +use std::sync::atomic::AtomicBool; +use std::sync::atomic::Ordering; +use std::time::Duration; +use std::time::Instant; + +use lore_error_set::prelude::*; +use lore_revision::lore_debug; + +use crate::remote::network::UdsStream; +use crate::remote::network::uds_supported; + +#[error_set] +pub enum ServiceProcessError {} + +/// How long `ensure_running` waits for a freshly spawned service to bind its +/// socket before giving up. +const START_TIMEOUT: Duration = Duration::from_secs(10); +const START_POLL_INTERVAL: Duration = Duration::from_millis(50); +/// How long `wait_until_stopped` gives the service to release its socket. The +/// service itself bounds its shutdown at five seconds, so this allows for that +/// plus the time to unwind. +const STOP_TIMEOUT: Duration = Duration::from_secs(10); + +/// Set by the service process itself, holding the flag its accept loop watches. +/// Its presence is also what tells a command handler that it is executing +/// inside the service rather than in a client. +static SHUTDOWN_FLAG: OnceLock> = OnceLock::new(); + +/// Called by the service process at startup to publish the flag that stops its +/// accept loop, so that a `ServiceStop` arriving over IPC can trip it. +pub fn register_shutdown_flag(flag: Arc) { + let _ = SHUTDOWN_FLAG.set(flag); +} + +pub fn running_as_service() -> bool { + SHUTDOWN_FLAG.get().is_some() +} + +/// Stops this process's own service loop. The accept loop is blocked in +/// `accept`, so it also needs a connection to wake it before it can observe the +/// flag; a failure to make that connection leaves the loop parked, so it is +/// reported rather than ignored. +pub fn request_shutdown() -> Result<(), ServiceProcessError> { + let Some(flag) = SHUTDOWN_FLAG.get() else { + return Err(ServiceProcessError::internal( + "this process is not running as a service", + )); + }; + lore_debug!("Stopping Lore service process"); + flag.store(true, Ordering::SeqCst); + UdsStream::connect().forward::("waking the accept loop")?; + Ok(()) +} + +pub fn is_running() -> bool { + uds_supported() && UdsStream::connect().is_ok() +} + +/// The executable to relaunch as the service. +/// +/// Auto-start only fires for the Lore CLI. When Lore is embedded as a library +/// the current executable is the host application, and running it with +/// `service run` would launch something arbitrary, so that case is refused and +/// the caller is told to start the service itself. +fn service_executable() -> Result { + let path = std::env::current_exe().internal("resolving the current executable")?; + let name = path + .file_stem() + .and_then(|stem| stem.to_str()) + .unwrap_or_default(); + if !name.eq_ignore_ascii_case("lore") { + return Err(ServiceProcessError::internal(format!( + "cannot start the Lore service automatically from {}; run `lore service start` instead", + path.display() + ))); + } + Ok(path) +} + +/// Spawns a detached `lore service run`. The child keeps running after this +/// process exits, so it is given no console and no inherited standard streams. +pub fn spawn() -> Result<(), ServiceProcessError> { + let executable = service_executable()?; + lore_debug!("Starting Lore service process: {}", executable.display()); + + let mut command = Command::new(&executable); + command + .arg("service") + .arg("run") + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()); + + #[cfg(target_family = "unix")] + { + use std::os::unix::process::CommandExt; + // Safety: setsid is async-signal-safe and is the documented way to + // detach the child from the caller's session and controlling terminal. + unsafe { + command.pre_exec(|| { + libc::setsid(); + Ok(()) + }); + } + } + + #[cfg(target_os = "windows")] + { + use std::os::windows::process::CommandExt; + const DETACHED_PROCESS: u32 = 0x0000_0008; + const CREATE_NO_WINDOW: u32 = 0x0800_0000; + command.creation_flags(DETACHED_PROCESS | CREATE_NO_WINDOW); + } + + command + .spawn() + .internal_with(|| format!("spawning {}", executable.display()))?; + + Ok(()) +} + +/// Makes sure a service process is listening, starting one if it is not. +/// Returns `true` if a new process was spawned. +pub async fn ensure_running() -> Result { + if !uds_supported() { + return Err(ServiceProcessError::internal( + "the Lore service is not supported on this OS", + )); + } + if is_running() { + return Ok(false); + } + + spawn()?; + + let deadline = Instant::now() + START_TIMEOUT; + while Instant::now() < deadline { + if is_running() { + lore_debug!("Lore service process is listening"); + return Ok(true); + } + tokio::time::sleep(START_POLL_INTERVAL).await; + } + + Err(ServiceProcessError::internal(format!( + "the Lore service did not start listening within {} seconds", + START_TIMEOUT.as_secs() + ))) +} + +/// Waits for a stopping service to stop answering on its socket. Returns +/// `false` if it is still listening when the timeout expires. +pub async fn wait_until_stopped() -> bool { + let deadline = Instant::now() + STOP_TIMEOUT; + while Instant::now() < deadline { + if !is_running() { + return true; + } + tokio::time::sleep(START_POLL_INTERVAL).await; + } + !is_running() +} diff --git a/lore/src/service.rs b/lore/src/service.rs index 1b6c0462..a772d5d5 100644 --- a/lore/src/service.rs +++ b/lore/src/service.rs @@ -1,20 +1,34 @@ // SPDX-FileCopyrightText: 2026 Epic Games, Inc. // SPDX-License-Identifier: MIT +use lore_error_set::prelude::*; use lore_macro::LoreArgs; +use lore_revision::event::EventError; +use lore_revision::global::GlobalConfig; use lore_revision::interface::LoreGlobalArgs; use serde::Deserialize; use serde::Serialize; -use crate::call_delegation::dispatch_call; +use crate::call::no_repository_call; +use crate::call_delegation::invalidate_use_service_cache; use crate::interface::LoreEventCallback; +use crate::remote::call::service_send_no_reply; +use crate::remote::process; + +#[error_set] +pub enum ServiceError {} + +impl EventError for ServiceError {} #[repr(C)] #[derive(Debug, Clone, PartialEq, Default, Serialize, Deserialize, LoreArgs)] #[handler(start_local)] -/// Arguments for starting the Lore service process for the current repository (no parameters). +/// Arguments for starting the Lore service process (no parameters). pub struct LoreServiceStartArgs {} -/// Start the Lore service process to manage the current repository. +/// Start the Lore service process, if it is not already running. +/// +/// Always runs locally rather than being dispatched to the service, which +/// cannot be asked to start itself while it is not running. /// /// # Events /// @@ -28,41 +42,41 @@ pub struct LoreServiceStartArgs {} /// | [`LoreEvent::Error`](crate::interface::LoreEvent::Error) | Emitted for a non-fatal error during the operation | /// | [`LoreEvent::Complete`](crate::interface::LoreEvent::Complete) | Always emitted at the end; `status` is `0` on success or the error code on failure | /// | [`LoreEvent::End`](crate::interface::LoreEvent::End) | Always emitted after `Complete` to signal callback termination | -#[allow(clippy::unused_async)] pub async fn start( globals: LoreGlobalArgs, args: LoreServiceStartArgs, callback: LoreEventCallback, ) -> i32 { - dispatch_call(globals, args, callback, start_local).await + start_local(globals, args, callback).await } async fn start_local( - _globals: LoreGlobalArgs, - _args: LoreServiceStartArgs, - _callback: LoreEventCallback, + globals: LoreGlobalArgs, + args: LoreServiceStartArgs, + callback: LoreEventCallback, ) -> i32 { - // Set sentinel in repository that it is being controlled by service process - - // Attempt to connect to service process - - // If fail, try starting a new service process and connect again - - // Send a message to service the given repository - - 1 + let command = async move |_args| -> Result<(), ServiceError> { + if process::running_as_service() { + return Ok(()); + } + process::ensure_running() + .await + .forward::("starting the Lore service")?; + Ok(()) + }; + no_repository_call(globals, callback, args, "service start", command).await } #[repr(C)] #[derive(Debug, Clone, PartialEq, Default, Serialize, Deserialize, LoreArgs)] #[handler(stop_local)] -/// Arguments for stopping the Lore service process for the current or all repositories. -pub struct LoreServiceStopArgs { - /// Stop all repositories rather than just the current one - pub all: u8, -} +/// Arguments for stopping the Lore service process (no parameters). +pub struct LoreServiceStopArgs {} -/// Stop the Lore service process for the current or all repositories. +/// Stop the Lore service process, if it is running. +/// +/// Always runs locally rather than being dispatched to the service, which would +/// start a service in order to deliver the request to stop it. /// /// # Events /// @@ -76,25 +90,103 @@ pub struct LoreServiceStopArgs { /// | [`LoreEvent::Error`](crate::interface::LoreEvent::Error) | Emitted for a non-fatal error during the operation | /// | [`LoreEvent::Complete`](crate::interface::LoreEvent::Complete) | Always emitted at the end; `status` is `0` on success or the error code on failure | /// | [`LoreEvent::End`](crate::interface::LoreEvent::End) | Always emitted after `Complete` to signal callback termination | -#[allow(clippy::unused_async)] pub async fn stop( globals: LoreGlobalArgs, args: LoreServiceStopArgs, callback: LoreEventCallback, ) -> i32 { - dispatch_call(globals, args, callback, stop_local).await + stop_local(globals, args, callback).await } async fn stop_local( - _globals: LoreGlobalArgs, - _args: LoreServiceStopArgs, - _callback: LoreEventCallback, + globals: LoreGlobalArgs, + args: LoreServiceStopArgs, + callback: LoreEventCallback, ) -> i32 { - // Attempt to connect to service process + let globals_for_send = globals.clone(); + let command = async move |args| -> Result<(), ServiceError> { + if process::running_as_service() { + process::request_shutdown().forward::("stopping the Lore service")?; + return Ok(()); + } + + if !process::is_running() { + return Ok(()); + } - // If successful, send a message to service the given repository + service_send_no_reply(globals_for_send, args) + .await + .forward::("sending stop to the Lore service")?; - // Remove sentinel in repository so that it is no longer being controlled by service process + if !process::wait_until_stopped().await { + return Err(ServiceError::internal( + "the Lore service did not stop in time", + )); + } + Ok(()) + }; + no_repository_call(globals, callback, args, "service stop", command).await +} - 1 +#[repr(C)] +#[derive(Debug, Clone, PartialEq, Default, Serialize, Deserialize, LoreArgs)] +#[handler(set_use_automatically_local)] +/// Arguments for setting whether Lore automatically routes calls through the service process. +pub struct LoreServiceSetUseAutomaticallyArgs { + /// Automatically use the service process + pub enabled: u8, +} + +/// Sets whether Lore automatically routes calls through the service process. +/// +/// Always runs locally rather than being dispatched to the service, which would +/// start a service only to be told that the service should no longer be used. +/// +/// # Events +/// +/// ## Standard Events +/// +/// These events are emitted by all interface functions: +/// +/// | Event | Description | +/// |-------|-------------| +/// | [`LoreEvent::Log`](crate::interface::LoreEvent::Log) | Diagnostic messages throughout execution | +/// | [`LoreEvent::Error`](crate::interface::LoreEvent::Error) | Emitted for a non-fatal error during the operation | +/// | [`LoreEvent::Complete`](crate::interface::LoreEvent::Complete) | Always emitted at the end; `status` is `0` on success or the error code on failure | +/// | [`LoreEvent::End`](crate::interface::LoreEvent::End) | Always emitted after `Complete` to signal callback termination | +pub async fn set_use_automatically( + globals: LoreGlobalArgs, + args: LoreServiceSetUseAutomaticallyArgs, + callback: LoreEventCallback, +) -> i32 { + set_use_automatically_local(globals, args, callback).await +} + +async fn set_use_automatically_local( + globals: LoreGlobalArgs, + args: LoreServiceSetUseAutomaticallyArgs, + callback: LoreEventCallback, +) -> i32 { + let command = + async move |args: LoreServiceSetUseAutomaticallyArgs| -> Result<(), ServiceError> { + let (mut config, lock) = GlobalConfig::load_locked() + .await + .internal("loading global config")?; + if args.enabled != 0 { + config.use_service_automatically = Some(true); + } else { + config.use_service_automatically = None; + } + config.save(lock).await.internal("saving global config")?; + invalidate_use_service_cache(); + Ok(()) + }; + no_repository_call( + globals, + callback, + args, + "service set-use-automatically", + command, + ) + .await } diff --git a/scripts/test/lore.py b/scripts/test/lore.py index 81ee1df9..2f363771 100644 --- a/scripts/test/lore.py +++ b/scripts/test/lore.py @@ -2256,8 +2256,16 @@ def service_run(self, **kwargs: Unpack[GlobalOptions]): def service_start(self, **kwargs: Unpack[GlobalOptions]): return self.run(["service", "start"], **kwargs) - def service_stop(self, stop_all: bool = False, **kwargs: Unpack[GlobalOptions]): - return self.run(["service", "stop", "true" if stop_all else "false"], **kwargs) + def service_stop(self, **kwargs: Unpack[GlobalOptions]): + return self.run(["service", "stop"], **kwargs) + + def service_set_use_automatically( + self, enabled: bool, **kwargs: Unpack[GlobalOptions] + ): + return self.run( + ["service", "set-use-automatically", "true" if enabled else "false"], + **kwargs, + ) def notification_subscribe( self, timeout: int | None = None, **kwargs: Unpack[GlobalOptions] diff --git a/scripts/test/test_service.py b/scripts/test/test_service.py index a09d6c7d..2a127d44 100644 --- a/scripts/test/test_service.py +++ b/scripts/test/test_service.py @@ -3,10 +3,10 @@ import logging import os import platform +import subprocess import pytest -from error_types import ServiceCallError from lore import Lore logger = logging.getLogger(__name__) @@ -18,15 +18,74 @@ def service_supported(): return platform.system() in ("Windows", "Linux", "Darwin") +def run_lore(lore_executable_path, args, global_dir): + """Runs the Lore CLI with an isolated global config directory.""" + environment = os.environ.copy() + environment["LORE_GLOBAL_PATH"] = global_dir + return subprocess.run( + [lore_executable_path, *args], + capture_output=True, + text=True, + env=environment, + ) + + +@pytest.fixture +def stopped_service(lore_executable_path, global_dir_name): + """Leaves no service process behind. + + The socket is per user rather than per test, so a service surviving one + test would be picked up by the next one. + """ + yield + run_lore(lore_executable_path, ["service", "stop"], global_dir_name) + + @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() ) -def test_service_down(new_lore_repo): - with pytest.raises(ServiceCallError): - new_lore_repo(environment_vars=LORE_SERVICE_ENVIRONMENT.copy()) +def test_service_start_stop(lore_executable_path, global_dir_name, stopped_service): + start = run_lore(lore_executable_path, ["service", "start"], global_dir_name) + assert start.returncode == 0, start.stdout + start.stderr + + # Starting again is a no-op rather than an error, because a service is + # already listening. + again = run_lore(lore_executable_path, ["service", "start"], global_dir_name) + assert again.returncode == 0, again.stdout + again.stderr + + stop = run_lore(lore_executable_path, ["service", "stop"], global_dir_name) + assert stop.returncode == 0, stop.stdout + stop.stderr + + # Stopping when nothing is running is also a no-op. + stop_again = run_lore(lore_executable_path, ["service", "stop"], global_dir_name) + assert stop_again.returncode == 0, stop_again.stdout + stop_again.stderr + + +@pytest.mark.smoke +@pytest.mark.skipif( + not service_supported(), reason="Service not supported on " + platform.system() +) +def test_service_set_use_automatically(lore_executable_path, global_dir_name): + config_path = os.path.join(global_dir_name, "config", "config.toml") + + enable = run_lore( + lore_executable_path, + ["service", "set-use-automatically", "true"], + global_dir_name, + ) + assert enable.returncode == 0, enable.stdout + enable.stderr + with open(config_path, encoding="utf-8") as config_file: + assert "use_service_automatically = true" in config_file.read() + + disable = run_lore( + lore_executable_path, + ["service", "set-use-automatically", "false"], + global_dir_name, + ) + assert disable.returncode == 0, disable.stdout + disable.stderr + with open(config_path, encoding="utf-8") as config_file: + assert "use_service_automatically" not in config_file.read() @pytest.mark.smoke @@ -123,3 +182,26 @@ def test_service_resolves_relative_paths_against_caller( assert "A " + file_name in map( lambda line: line.strip(" "), status_output.splitlines() ), f"Staged file should show as added: {status_output}" + + +@pytest.mark.smoke +@pytest.mark.skipif( + not service_supported(), reason="Service not supported on " + platform.system() +) +def test_service_starts_on_demand(new_lore_repo, stopped_service): + """A call routed to the service starts one when none is running. + + This replaces an older test asserting the opposite: before automatic + start-up, the same call failed with a connection error. + """ + repo: Lore = new_lore_repo(environment_vars=LORE_SERVICE_ENVIRONMENT.copy()) + + file_name = "test.uasset" + with repo.open_file(file_name, "w+b") as output_file: + output_file.write(os.urandom(30)) + + repo.stage(scan=True) + + assert "A " + file_name in map( + lambda line: line.strip(" "), repo.status().splitlines() + ) From 2f6188878759e0bcea7629bc5c3f7ee7bad89b54 Mon Sep 17 00:00:00 2001 From: Mattias Jansson Date: Mon, 20 Jul 2026 13:52:56 +0200 Subject: [PATCH 02/20] Size client threading for relaying to the service # Summary A client whose calls all execute in the service process still built the full runtime: on a ten-core machine that is ten tokio worker threads and a nine-thread rayon compute pool, both spawned eagerly, for a process that only writes one message and reads the reply. Measured at 25 threads for a single `lore status`. Such a process now sizes its runtime for relaying instead, which measures at 5 threads for the same command in the same repository, with no rayon threads at all. A client that is not relaying is unaffected and still gets the full runtime. ## Changes - `lore-base/src/runtime.rs`: `TokioSettings` gains `eager_compute_pool`, and `TokioSettings::relay_only()` for a process that performs no work of its own. Turning the compute pool off only stops it being paid for up front; `compute_pool()` still builds it on first use, so a caller that unexpectedly needs it gets it rather than failing. Both pools keep `MIN_THREADS_PER_POOL`, so neither can starve the other. - `lore/src/call_delegation.rs`: `will_use_service` answers the routing question without a runtime, by reading the global config directly rather than through its async loader. It fills the same cache `use_service` uses, so the decision taken at start-up and the one taken per call cannot disagree. - `lore/src/lib.rs`: exposes `will_use_service` and `size_threads_for_relaying`. - `lore-client/src/cli/client_main.rs`: sizes for relaying when the command will be routed to the service. `service run` is excluded: it is the process doing the work, so it keeps the full complement however the calling user has configured routing. The choice has to be made before the first Lore operation, because the runtime is built on first use and later settings are ignored. `log::initialize` and `setup_config` do not touch it, so the existing `set_thread_limit` call site is early enough. # Test Plan Peak thread counts, measured in a real repository so that the command does enough work to be sampled, three runs each and stable across them: | Client path | Threads | rayon (`lore-compute`) | | --- | --- | --- | | Executing locally | 25 | full pool | | Relaying over IPC | 5 | 0 | - That rayon is absent when relaying was confirmed by thread name rather than inferred from the count: `sample` on a client frozen mid-call reports zero `lore-compute` threads. The only lazy callers of `compute_pool` are in `lore-storage`, which a relaying client never reaches, because `dispatch_call` diverts at the API entry before any store is opened. - `service run` is unaffected at 23 threads, with all nine `lore-compute` threads present. - `lore status` output is byte-identical whether relayed or executed locally, and `service stop` works through the relaying runtime. - `cargo clippy --all-targets -- -D warnings --no-deps`, `cargo +nightly fmt --all --check`, `cargo test -p lore --lib` (90), `cargo test -p lore-base --lib` (79): all clean. Note on method: sampling `ps -M` against a bare `lore status` is unreliable, because the process is too short-lived to catch and reports whatever count happens to exist at that instant. The figures above come from freezing the client with SIGSTOP, and from running in a repository where the work lasts long enough to sample. Not verified on Windows. Signed-off-by: Mattias Jansson --- lore-base/src/runtime.rs | 37 ++++++++++++++++++++++++++---- lore-client/src/cli/client_main.rs | 16 +++++++++++++ lore/src/call_delegation.rs | 26 +++++++++++++++++++++ lore/src/lib.rs | 17 ++++++++++++++ 4 files changed, 92 insertions(+), 4 deletions(-) diff --git a/lore-base/src/runtime.rs b/lore-base/src/runtime.rs index 59ad7430..b57f22f4 100644 --- a/lore-base/src/runtime.rs +++ b/lore-base/src/runtime.rs @@ -541,6 +541,15 @@ pub struct TokioSettings { #[serde(default = "default_thread_keep_alive")] pub thread_keep_alive_seconds: u64, pub worker_threads: Option, + /// Whether to build the rayon compute pool up front. Turning this off does + /// not make the pool unavailable: [`compute_pool`] still builds it on first + /// use, so this only decides whether its threads are paid for eagerly. + #[serde(default = "default_eager_compute_pool")] + pub eager_compute_pool: bool, +} + +fn default_eager_compute_pool() -> bool { + true } impl Default for TokioSettings { @@ -549,6 +558,23 @@ impl Default for TokioSettings { max_blocking_threads: default_blocking_threads(), thread_keep_alive_seconds: default_thread_keep_alive(), worker_threads: None, + eager_compute_pool: true, + } + } +} + +impl TokioSettings { + /// Settings for a process that only relays work elsewhere, such as a client + /// whose calls all execute in the Lore service. Sized for IPC rather than + /// for doing the work, and with no compute pool, which such a process never + /// touches. Each pool keeps [`MIN_THREADS_PER_POOL`] so that neither can + /// starve the other. + pub fn relay_only() -> Self { + TokioSettings { + max_blocking_threads: MIN_THREADS_PER_POOL, + thread_keep_alive_seconds: default_thread_keep_alive(), + worker_threads: Some(MIN_THREADS_PER_POOL), + eager_compute_pool: false, } } } @@ -600,10 +626,12 @@ pub fn runtime_with_settings(settings: Option) -> Handle { // Build the compute pool off-thread so runtime creation isn't // blocked on spawning N rayon workers. No LORE_CONTEXT is active // yet, so Handle::spawn directly rather than lore_spawn!. - #[allow(clippy::disallowed_methods)] - handle.spawn(async { - let _ = COMPUTE_POOL.get_or_init(build_compute_pool); - }); + if settings.eager_compute_pool { + #[allow(clippy::disallowed_methods)] + handle.spawn(async { + let _ = COMPUTE_POOL.get_or_init(build_compute_pool); + }); + } handle } @@ -693,6 +721,7 @@ mod tests { max_blocking_threads: 4, thread_keep_alive_seconds: 5, worker_threads: Some(2), + eager_compute_pool: true, }; let handle = runtime_with_settings(Some(settings)); handle.block_on(async { diff --git a/lore-client/src/cli/client_main.rs b/lore-client/src/cli/client_main.rs index c355ef8e..c46f71ae 100644 --- a/lore-client/src/cli/client_main.rs +++ b/lore-client/src/cli/client_main.rs @@ -5,11 +5,23 @@ use clap::CommandFactory; use clap::Parser; use crate::cli::LoreCli; +use crate::cli::LoreCommands; use crate::cli::handle_lore_commands; use crate::cli::lore_globals_from_args; +use crate::commands::service::ServiceCommands; use crate::config::setup_config; use crate::logging; +/// Whether this command does its own work rather than relaying it to the Lore +/// service. Only `service run` does: it *is* the service, so it needs the full +/// complement of threads however the calling user has configured routing. +fn runs_work_in_this_process(command: &LoreCommands) -> bool { + matches!( + command, + LoreCommands::Service(args) if matches!(args.command, ServiceCommands::Run(_)) + ) +} + pub fn client_main() -> ExitCode { #[cfg(target_family = "windows")] // safety: safe Win32 call; no invariants to uphold @@ -56,6 +68,10 @@ pub fn client_main() -> ExitCode { lore::set_thread_limit(max_threads); } + if !runs_work_in_this_process(cli_command) && lore::will_use_service() { + lore::size_threads_for_relaying(); + } + let globals = lore_globals_from_args(&cli); let result = handle_lore_commands(cli_command, globals); diff --git a/lore/src/call_delegation.rs b/lore/src/call_delegation.rs index a9f6c413..a43c2e49 100644 --- a/lore/src/call_delegation.rs +++ b/lore/src/call_delegation.rs @@ -1,6 +1,8 @@ // SPDX-FileCopyrightText: 2026 Epic Games, Inc. // SPDX-License-Identifier: MIT +use lore_revision::global::CONFIG as GLOBAL_CONFIG; use lore_revision::global::GlobalConfig; +use lore_revision::global::get_global_config_dir; use lore_revision::interface::LoreGlobalArgs; use lore_revision::lore_debug; use parking_lot::RwLock; @@ -60,6 +62,30 @@ pub(crate) async fn use_service() -> bool { enabled } +/// Answers the same question as [`use_service`] without a runtime, for callers +/// that must decide before one exists. Reads the global config directly rather +/// than through its async loader, and fills the same cache, so a later +/// [`use_service`] cannot reach a different answer. +pub fn will_use_service() -> bool { + if let Some(value) = use_service_override() { + return value; + } + if cfg!(test) { + return false; + } + if let Some(cached) = *USE_SERVICE.read() { + return cached; + } + let enabled = get_global_config_dir() + .ok() + .map(|dir| dir.join(GLOBAL_CONFIG)) + .and_then(|path| std::fs::read_to_string(path).ok()) + .and_then(|text| toml::from_str::(&text).ok()) + .is_some_and(|config| config.use_service_automatically()); + *USE_SERVICE.write() = Some(enabled); + enabled +} + pub(crate) fn run_synchronously< ArgsType: InvokableLoreArgs + Clone + Send + 'static, Handler: Fn(LoreGlobalArgs, ArgsType, LoreEventCallback) -> Fut, diff --git a/lore/src/lib.rs b/lore/src/lib.rs index 9bcbd590..41271b8a 100644 --- a/lore/src/lib.rs +++ b/lore/src/lib.rs @@ -79,6 +79,23 @@ pub fn set_thread_limit(count: usize) -> bool { lore_base::runtime::set_thread_limit(count) } +/// Whether calls will be executed by the Lore service process rather than in +/// this one. Safe to call before a runtime exists, so a caller can size its +/// threading before doing any work. See [`size_threads_for_relaying`]. +pub fn will_use_service() -> bool { + call_delegation::will_use_service() +} + +/// Sizes the shared runtime for a process that only relays its work to the Lore +/// service, rather than performing it. Creates the runtime, so it must be +/// called before the first Lore operation and only by a process that does no +/// work of its own; the service process itself must never call it. +pub fn size_threads_for_relaying() { + drop(lore_base::runtime::runtime_with_settings(Some( + lore_base::runtime::TokioSettings::relay_only(), + ))); +} + pub fn log_file_path() -> LoreString { log::get_logs_path().into() } From 85e5b2dcd535a05e3c364fbeeddb6dcdc7a0e4a7 Mon Sep 17 00:00:00 2001 From: Mattias Jansson Date: Mon, 20 Jul 2026 21:38:55 +0200 Subject: [PATCH 03/20] Pin the service tests to a single test worker The suite runs under pytest-xdist with `--dist loadgroup`, so tests are spread across workers unless grouped. The service tests share one per-user socket, and two of them start or stop the service, so running them on different workers at once would let one bind while another unlinks. Group them onto a single worker, matching how the other stateful suites (topology, replicated store, forwarded requests) already do it. - `scripts/test/test_service.py`: add `@pytest.mark.xdist_group("lore_service")` to all four service tests. - `uv run pytest scripts/test/test_service.py -v`: 4 passed. Alongside pinning them, this is the first end-to-end run of the start/stop feature against a real build, covering automatic start-up, explicit start and stop, and the use-automatically setting. Signed-off-by: Mattias Jansson --- scripts/test/test_service.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/test/test_service.py b/scripts/test/test_service.py index 2a127d44..701ccac6 100644 --- a/scripts/test/test_service.py +++ b/scripts/test/test_service.py @@ -42,6 +42,7 @@ def stopped_service(lore_executable_path, global_dir_name): @pytest.mark.smoke +@pytest.mark.xdist_group("lore_service") @pytest.mark.skipif( not service_supported(), reason="Service not supported on " + platform.system() ) @@ -63,6 +64,7 @@ def test_service_start_stop(lore_executable_path, global_dir_name, stopped_servi @pytest.mark.smoke +@pytest.mark.xdist_group("lore_service") @pytest.mark.skipif( not service_supported(), reason="Service not supported on " + platform.system() ) @@ -185,6 +187,7 @@ def test_service_resolves_relative_paths_against_caller( @pytest.mark.smoke +@pytest.mark.xdist_group("lore_service") @pytest.mark.skipif( not service_supported(), reason="Service not supported on " + platform.system() ) From 0f392ea33520948f72039ec8328c3d68bbcaa267 Mon Sep 17 00:00:00 2001 From: Mattias Jansson Date: Wed, 22 Jul 2026 21:54:19 +0200 Subject: [PATCH 04/20] Test the client runtime sizing for relaying vs local work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Summary The choice of runtime size — lean when a client only relays to the service, full otherwise — had no automated coverage. Add unit tests for the sizing, and refactor the runtime builder so the sizing can be asserted without the shared runtime singleton. ## Changes - `lore-base/src/runtime.rs`: extract `resolve_worker_threads` and `build_tokio_runtime` from `runtime_with_settings`, so a runtime can be built from a `TokioSettings` in isolation. Behaviour is unchanged. Tests: - `relay_only_settings_are_minimal`: the relay settings use the per-pool minimum and do not build the compute pool eagerly, while the default does. - `relay_runtime_is_smaller_than_full`: a runtime built from `relay_only()` has the minimum worker count, and the full runtime is never smaller. - `built_runtime_honors_resolved_worker_count`: the built runtime's worker count matches what `resolve_worker_threads` resolves, independent of the `LORE_WORKER_THREADS` override. - `lore-client/src/cli/client_main.rs`: test `runs_work_in_this_process` — only `service run` keeps the full runtime; `service start`/`stop`/ `set-use-automatically` and a regular command all relay and are sized lean. This pins the exclusion that keeps the service process itself full. # Test Plan - `cargo test -p lore-base --lib` (82) and `cargo test -p lore-client --lib` (5): all pass. - `cargo clippy --all-targets -- -D warnings --no-deps` and `cargo +nightly fmt --all --check`: clean. Not covered by these unit tests, and noted for follow-up: the end-to-end wiring (that `client_main` actually applies `relay_only` when routing to the service) and the absence of rayon threads in a relaying client were verified by hand (worker threads 25 -> 5, zero compute threads), not in an automated test, and no CI currently runs any of this. Signed-off-by: Mattias Jansson --- lore-base/src/runtime.rs | 104 +++++++++++++++++++++++------ lore-client/src/cli/client_main.rs | 35 ++++++++++ 2 files changed, 117 insertions(+), 22 deletions(-) diff --git a/lore-base/src/runtime.rs b/lore-base/src/runtime.rs index b57f22f4..e55ce215 100644 --- a/lore-base/src/runtime.rs +++ b/lore-base/src/runtime.rs @@ -589,6 +589,38 @@ pub fn runtime() -> Handle { /// If no runtime exists yet, creates one with the provided settings (or defaults if `None`). /// If a tokio runtime is already active on the current thread, returns its handle instead. /// Respects the `LORE_WORKER_THREADS` environment variable for overriding worker thread count. +/// The number of tokio worker threads a runtime with these settings is built +/// with. Precedence: the `LORE_WORKER_THREADS` env override, then an explicit +/// positive count in the settings, then the budget-derived default. Always a +/// concrete count, because leaving it unset makes tokio use the raw core count +/// and ignore the thread limit. +fn resolve_worker_threads(settings: &TokioSettings) -> usize { + match ( + env_thread_override("LORE_WORKER_THREADS"), + settings.worker_threads, + ) { + (Some(val), _) => val, + (None, Some(val)) if val > 0 => val, + _ => default_worker_threads(), + } +} + +/// Builds a fresh multi-thread tokio runtime from the settings. Does not touch +/// the shared runtime or the compute pool, so it is safe to call in isolation. +fn build_tokio_runtime(settings: &TokioSettings) -> tokio::runtime::Runtime { + let mut builder = tokio::runtime::Builder::new_multi_thread(); + builder + .enable_all() + .max_blocking_threads(settings.max_blocking_threads) + .thread_keep_alive(Duration::from_secs(settings.thread_keep_alive_seconds)) + .thread_name_fn(|| { + static ID: AtomicUsize = AtomicUsize::new(0); + format!("lore-tokio-{}", ID.fetch_add(1, Ordering::Relaxed)) + }) + .worker_threads(resolve_worker_threads(settings)); + builder.build().expect("Failed to create runtime") +} + pub fn runtime_with_settings(settings: Option) -> Handle { if let Ok(handle) = tokio::runtime::Handle::try_current() { handle @@ -598,28 +630,7 @@ pub fn runtime_with_settings(settings: Option) -> Handle { runtime.handle().clone() } else { let settings = settings.unwrap_or_default(); - let mut builder = tokio::runtime::Builder::new_multi_thread(); - builder - .enable_all() - .max_blocking_threads(settings.max_blocking_threads) - .thread_keep_alive(Duration::from_secs(settings.thread_keep_alive_seconds)) - .thread_name_fn(|| { - static ID: AtomicUsize = AtomicUsize::new(0); - format!("lore-tokio-{}", ID.fetch_add(1, Ordering::Relaxed)) - }); - // Always set an explicit count, else tokio would default to the raw - // core count and ignore the thread limit. Precedence: env override, - // explicit setting, budget-derived default. - let worker_threads = match ( - env_thread_override("LORE_WORKER_THREADS"), - settings.worker_threads, - ) { - (Some(val), _) => val, - (None, Some(val)) if val > 0 => val, - _ => default_worker_threads(), - }; - builder.worker_threads(worker_threads); - let runtime = builder.build().expect("Failed to create runtime"); + let runtime = build_tokio_runtime(&settings); let handle = runtime.handle().clone(); *default_runtime = Some(runtime); @@ -737,6 +748,55 @@ mod tests { assert_eq!(counts.compute, 7); } + #[test] + fn relay_only_settings_are_minimal() { + let relay = TokioSettings::relay_only(); + assert_eq!(relay.worker_threads, Some(MIN_THREADS_PER_POOL)); + assert_eq!(relay.max_blocking_threads, MIN_THREADS_PER_POOL); + assert!( + !relay.eager_compute_pool, + "a relay process never touches the compute pool, so it must not build it eagerly" + ); + assert!( + TokioSettings::default().eager_compute_pool, + "a full runtime builds the compute pool eagerly" + ); + } + + #[test] + fn relay_runtime_is_smaller_than_full() { + // The env override, if set in the test environment, would defeat the + // per-settings worker count both branches resolve, so skip then. + if env_thread_override("LORE_WORKER_THREADS").is_some() { + return; + } + + let relay = build_tokio_runtime(&TokioSettings::relay_only()); + let full = build_tokio_runtime(&TokioSettings::default()); + + assert_eq!( + relay.metrics().num_workers(), + MIN_THREADS_PER_POOL, + "a relay runtime is sized for IPC, not for doing the work" + ); + assert!( + full.metrics().num_workers() >= relay.metrics().num_workers(), + "the full runtime is never smaller than the relay one" + ); + } + + #[test] + fn built_runtime_honors_resolved_worker_count() { + // Independent of any env override: whatever count is resolved is the + // count the runtime is actually built with. + let settings = TokioSettings::relay_only(); + let runtime = build_tokio_runtime(&settings); + assert_eq!( + runtime.metrics().num_workers(), + resolve_worker_threads(&settings) + ); + } + #[test] fn apportion_returns_defaults_when_within_limit() { let defaults = default_thread_counts(8); diff --git a/lore-client/src/cli/client_main.rs b/lore-client/src/cli/client_main.rs index c46f71ae..856df43f 100644 --- a/lore-client/src/cli/client_main.rs +++ b/lore-client/src/cli/client_main.rs @@ -84,3 +84,38 @@ pub fn client_main() -> ExitCode { return ExitCode::from(result); } + +#[cfg(test)] +mod tests { + use super::*; + + fn command(args: &[&str]) -> LoreCommands { + LoreCli::try_parse_from(args) + .expect("args should parse") + .command + .expect("a subcommand should be present") + } + + #[test] + fn only_service_run_does_work_in_this_process() { + // The service process itself keeps the full runtime. + assert!(runs_work_in_this_process(&command(&[ + "lore", "service", "run" + ]))); + + // Everything else relays to the service and must not be excluded, so it + // can be sized lean. The service-control commands run locally but are + // light, and a regular command routes through the service. + for relaying in [ + vec!["lore", "service", "start"], + vec!["lore", "service", "stop"], + vec!["lore", "service", "set-use-automatically", "true"], + vec!["lore", "status"], + ] { + assert!( + !runs_work_in_this_process(&command(&relaying)), + "{relaying:?} must not be treated as doing work in this process" + ); + } + } +} From 77e0c6113fd67b0b29fb5e5e082ec9583cfed771 Mon Sep 17 00:00:00 2001 From: Mattias Jansson Date: Wed, 22 Jul 2026 22:02:41 +0200 Subject: [PATCH 05/20] Isolate the service's global config in tests and cover config routing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Summary The `background_lore_service` fixture started the service without setting `LORE_GLOBAL_PATH`, so the service ran against the developer's real global config. A test routed through it — creating a shared store while serving a command — wrote the store entry into the real `config.toml`. Isolate the service the same way `lore_service_in_directory` already does, and add a test for the config-driven routing path, which had no coverage. ## Changes - `scripts/test/conftest.py`: `background_lore_service` now starts the service with the test's `LORE_GLOBAL_PATH`, and the readiness probe uses it too, so nothing the service writes lands in the real global config. - `scripts/test/test_service.py`: `test_config_setting_routes_to_service` enables `use_service_automatically` in the isolated config and shows a plain command then routes to the service without the `LORE_USE_SERVICE` override. Routing is proven without a real daemon: a binary not named `lore` refuses to auto-start the service, so a routed command fails with that refusal while the same binary forced local does not. This is the first coverage of the config-driven routing path — the feature's primary production trigger. # Test Plan - `uv run pytest scripts/test/test_service.py`: 6 passed. - Snapshotted the real global config before the run and confirmed it was byte-identical after (same mtime, same entries), where before this change a run added `default_shared_stores` entries to it. - `uv run ruff check`: clean. Signed-off-by: Mattias Jansson --- scripts/test/conftest.py | 18 +++++++--- scripts/test/test_service.py | 68 ++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 5 deletions(-) diff --git a/scripts/test/conftest.py b/scripts/test/conftest.py index f5ff3797..0aa0d5fc 100644 --- a/scripts/test/conftest.py +++ b/scripts/test/conftest.py @@ -181,10 +181,13 @@ def _service_unreachable(output): return platform.system() == "Windows" and "10022" in output -def _wait_for_service_ready(lore_executable_path, service_process, attempts=30): +def _wait_for_service_ready( + lore_executable_path, service_process, global_dir, attempts=30 +): """Block until the background service answers a probe.""" probe_env = os.environ.copy() probe_env["LORE_USE_SERVICE"] = "1" + probe_env["LORE_GLOBAL_PATH"] = global_dir for _ in range(attempts): if service_process.poll() is not None: pytest.fail( @@ -219,7 +222,7 @@ def start(directory): [lore_executable_path, "service", "run"], cwd=str(directory), env=env ) processes.append(service_process) - _wait_for_service_ready(lore_executable_path, service_process) + _wait_for_service_ready(lore_executable_path, service_process, global_dir_name) return service_process yield start @@ -234,12 +237,17 @@ def start(directory): @pytest.fixture(scope="function") -def background_lore_service(lore_executable_path): +def background_lore_service(lore_executable_path, global_dir_name): + # Give the service the test's isolated global config so that anything it + # writes there (for example a shared store created while serving a command) + # never lands in the developer's real global config. + env = os.environ.copy() + env["LORE_GLOBAL_PATH"] = global_dir_name command_args = [lore_executable_path, "service", "run"] logger.info("Executing Lore service command: %s", command_args) - service_process = subprocess.Popen(command_args) + service_process = subprocess.Popen(command_args, env=env) - _wait_for_service_ready(lore_executable_path, service_process) + _wait_for_service_ready(lore_executable_path, service_process, global_dir_name) yield service_process diff --git a/scripts/test/test_service.py b/scripts/test/test_service.py index 701ccac6..5751bc99 100644 --- a/scripts/test/test_service.py +++ b/scripts/test/test_service.py @@ -3,6 +3,7 @@ import logging import os import platform +import shutil import subprocess import pytest @@ -90,6 +91,73 @@ def test_service_set_use_automatically(lore_executable_path, global_dir_name): assert "use_service_automatically" not in config_file.read() +@pytest.mark.smoke +@pytest.mark.xdist_group("lore_service") +@pytest.mark.skipif( + not service_supported(), reason="Service not supported on " + platform.system() +) +def test_config_setting_routes_to_service( + lore_executable_path, global_dir_name, tmp_path, stopped_service +): + """The use_service_automatically config setting routes commands through the + service, without the LORE_USE_SERVICE override. + + Everything runs against the test's isolated LORE_GLOBAL_PATH, so the + developer's real global config is never read or written. Routing is proven + without starting a real daemon: a binary not named `lore` refuses to + auto-start the service, so a command that tries to route fails with that + refusal, while the same binary forced to run locally does not. + """ + refusal = "start the Lore service automatically" + + # No service must be listening, so a routed command takes the auto-start + # path where a non-`lore` binary refuses. + run_lore(lore_executable_path, ["service", "stop"], global_dir_name) + + enable = run_lore( + lore_executable_path, + ["service", "set-use-automatically", "true"], + global_dir_name, + ) + assert enable.returncode == 0, enable.stdout + enable.stderr + + binary_name = "notlore.exe" if platform.system() == "Windows" else "notlore" + not_lore = tmp_path / binary_name + shutil.copy(lore_executable_path, not_lore) + not_lore.chmod(0o755) + + env = os.environ.copy() + env["LORE_GLOBAL_PATH"] = global_dir_name + + routed = subprocess.run( + [str(not_lore), "status"], + capture_output=True, + text=True, + cwd=str(tmp_path), + env=env, + ) + assert refusal in (routed.stdout + routed.stderr), ( + "the config setting should have routed the command to the service, " + f"got: {routed.stdout}{routed.stderr}" + ) + + # Control: the same binary forced to run locally does not try to reach the + # service, so the failure above was the routing decision, not the rename. + local_env = env.copy() + local_env["LORE_USE_SERVICE"] = "0" + local = subprocess.run( + [str(not_lore), "status"], + capture_output=True, + text=True, + cwd=str(tmp_path), + env=local_env, + ) + assert refusal not in (local.stdout + local.stderr), ( + f"forcing local execution should not route to the service: " + f"{local.stdout}{local.stderr}" + ) + + @pytest.mark.smoke @pytest.mark.xdist_group("lore_service") @pytest.mark.skipif( From f86aacb8c6a01c6e5201a285db28a4528d30fc4d Mon Sep 17 00:00:00 2001 From: Mattias Jansson Date: Wed, 22 Jul 2026 22:20:46 +0200 Subject: [PATCH 06/20] Recognise more off values for LORE_USE_SERVICE # Summary `LORE_USE_SERVICE` only treated an empty value and `0`/`false` as off, so a user exporting `off` or `no` to disable service routing silently enabled it, and values were not trimmed. Recognise the common off tokens and ignore surrounding whitespace. ## Changes - `lore/src/call_delegation.rs`: extract `use_service_from_value`, which treats an empty value and `0`, `false`, `f`, `no`, `n`, and `off` (case-insensitive, trimmed) as off and any other value as on, and unit-test it directly without touching the process environment. # Test Plan - `cargo test -p lore --lib call_delegation`: passes, including the new value-parsing test. - `cargo clippy -p lore --all-targets -- -D warnings --no-deps` and `cargo +nightly fmt --all --check`: clean. Signed-off-by: Mattias Jansson --- lore/src/call_delegation.rs | 38 +++++++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/lore/src/call_delegation.rs b/lore/src/call_delegation.rs index a43c2e49..e216171c 100644 --- a/lore/src/call_delegation.rs +++ b/lore/src/call_delegation.rs @@ -27,15 +27,23 @@ pub(crate) fn invalidate_use_service_cache() { *USE_SERVICE.write() = None; } +/// Whether a `LORE_USE_SERVICE` value turns the service on. Off values, matched +/// case-insensitively with surrounding whitespace ignored, are an empty value +/// and `0`, `false`, `f`, `no`, `n`, and `off`; any other value is on. +fn use_service_from_value(value: &str) -> bool { + !matches!( + value.trim().to_ascii_lowercase().as_str(), + "" | "0" | "false" | "f" | "no" | "n" | "off" + ) +} + /// `LORE_USE_SERVICE` overrides the stored setting when set, so that tests and /// one-off invocations can route through the service without writing config. -/// An empty value, or `0`/`false`, means "do not use the service". +/// See [`use_service_from_value`] for how the value is interpreted. fn use_service_override() -> Option { - let value = std::env::var(USE_SERVICE_VAR).ok()?; - if value.is_empty() { - return Some(false); - } - Some(!value.eq_ignore_ascii_case("0") && !value.eq_ignore_ascii_case("false")) + std::env::var(USE_SERVICE_VAR) + .ok() + .map(|value| use_service_from_value(&value)) } /// Whether calls should be routed through the service process. @@ -160,6 +168,24 @@ mod tests { use super::*; use crate::interface::LoreString; + #[test] + fn use_service_value_parsing() { + for on in ["1", "true", "TRUE", "yes", "on", "enabled", " 1 "] { + assert!( + use_service_from_value(on), + "{on:?} should enable the service" + ); + } + for off in [ + "", " ", "0", "false", "FALSE", "f", "no", "n", "off", " off ", + ] { + assert!( + !use_service_from_value(off), + "{off:?} should disable the service" + ); + } + } + // A concrete error whose `NotFound` variant carries error code 13, so the // async failure path has a known non-`1` code to assert against. #[error_set] From edd7929b0af9ecccddabc091319b7e2d1ae7aa24 Mon Sep 17 00:00:00 2001 From: Mattias Jansson Date: Wed, 22 Jul 2026 22:25:40 +0200 Subject: [PATCH 07/20] Treat a stopped service as success when stopping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Summary `service stop` was check-then-act: `is_running()`, then a send that connects to the socket, with an await between. If the service exited in that window — a concurrent stop or a termination signal — the send's connect failed and `stop` returned failure, even though the service was in fact stopped, which is the outcome asked for. ## Changes - `lore/src/service.rs`: in `stop_local`, a failed send is only an error if the service is still running afterwards; a service that has since stopped returns success. # Test Plan - Two `service stop` invocations racing against one live service, three trials: both return exit 0 every time and the service ends stopped. A normal stop and a stop against an already-stopped service both still return 0. - `cargo clippy -p lore --all-targets -- -D warnings --no-deps` and `cargo +nightly fmt --all --check`: clean. Signed-off-by: Mattias Jansson --- lore/src/service.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/lore/src/service.rs b/lore/src/service.rs index a772d5d5..507dc0f8 100644 --- a/lore/src/service.rs +++ b/lore/src/service.rs @@ -114,9 +114,16 @@ async fn stop_local( return Ok(()); } - service_send_no_reply(globals_for_send, args) - .await - .forward::("sending stop to the Lore service")?; + // The service can exit between the check above and the send below — + // another stop, or a termination signal. A failed send is then only an + // error if the service is somehow still running; a stopped service is + // exactly the outcome asked for. + if let Err(error) = service_send_no_reply(globals_for_send, args).await { + if process::is_running() { + return Err(error).forward::("sending stop to the Lore service"); + } + return Ok(()); + } if !process::wait_until_stopped().await { return Err(ServiceError::internal( From f6b0fcafd173b34cbd8d3bed3f32cbed837dfc4f Mon Sep 17 00:00:00 2001 From: Mattias Jansson Date: Wed, 22 Jul 2026 22:33:43 +0200 Subject: [PATCH 08/20] Document the use-service cache staleness limits # Summary The doc comment on the cache invalidation claimed a long-lived embedder never keeps the old value, which is only true for writes made by the same process. Document the actual limits: the cache is process-lifetime and does not see a change made by another process until restart, and the save-then-invalidate ordering races a concurrent reader. Both are acceptable for the CLI (one command per process) and only expose a long-lived multi-threaded embedder. ## Changes - `lore/src/call_delegation.rs`: correct the doc comments on the `USE_SERVICE` cache and `invalidate_use_service_cache`. # Test Plan - `cargo build -p lore` and `cargo +nightly fmt --all --check`: clean. Signed-off-by: Mattias Jansson --- lore/src/call_delegation.rs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/lore/src/call_delegation.rs b/lore/src/call_delegation.rs index e216171c..cb7e6443 100644 --- a/lore/src/call_delegation.rs +++ b/lore/src/call_delegation.rs @@ -18,11 +18,23 @@ const USE_SERVICE_VAR: &str = "LORE_USE_SERVICE"; /// Caches `use_service_automatically` for the life of the process. Every public /// API entry point consults it, and reading it means parsing the global TOML /// config, so it must not happen per call. +/// +/// The cache lives for the whole process and is only refreshed by a write in +/// this process (see [`invalidate_use_service_cache`]). A change made elsewhere +/// — an external `lore service set-use-automatically`, or a hand-edited config — +/// is not observed until the process restarts. That is acceptable because the +/// setting is a deploy-time choice that rarely changes under a running process, +/// and the CLI runs one command per process regardless. static USE_SERVICE: RwLock> = RwLock::new(None); -/// Drops the cached setting so the next call rereads it. Called after the -/// setting is written, so a long-lived embedder does not keep using the old -/// value. +/// Drops the cached setting so the next call in this process rereads it. Called +/// after the setting is written here. +/// +/// This refreshes only writes made by this process, and it races a concurrent +/// reader: a [`use_service`] that loads the old config and fills the cache after +/// this runs can leave the stale value in place until the next write. The window +/// is narrow and self-heals on any later write; only a long-lived multi-threaded +/// embedder that flips the setting mid-run is exposed. pub(crate) fn invalidate_use_service_cache() { *USE_SERVICE.write() = None; } From 1720e8411dc582457919c44f3809b5852b5fee95 Mon Sep 17 00:00:00 2001 From: Mattias Jansson Date: Wed, 22 Jul 2026 22:36:05 +0200 Subject: [PATCH 09/20] Document that relay sizing is one-shot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Summary The runtime is sized for relaying once, when it is built, and later settings are ignored. A long-lived process that turns the service off after sizing for relay then runs its work locally on the small runtime — correct but slow. Document this on `size_threads_for_relaying` so the constraint is explicit; the CLI is unaffected since it runs one command per process. ## Changes - `lore/src/lib.rs`: note the one-shot sizing and the flip-mid-process hazard on `size_threads_for_relaying`. # Test Plan - `cargo build -p lore`, `cargo doc -p lore --no-deps` (the new intra-doc link resolves), and `cargo +nightly fmt --all --check`: clean. Signed-off-by: Mattias Jansson --- lore/src/lib.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lore/src/lib.rs b/lore/src/lib.rs index 41271b8a..1d35a187 100644 --- a/lore/src/lib.rs +++ b/lore/src/lib.rs @@ -90,6 +90,13 @@ pub fn will_use_service() -> bool { /// service, rather than performing it. Creates the runtime, so it must be /// called before the first Lore operation and only by a process that does no /// work of its own; the service process itself must never call it. +/// +/// The sizing is applied once, when the runtime is built, and cannot be undone: +/// later settings are ignored because the runtime already exists. A long-lived +/// process that turns the service off after this has been called (for example +/// via [`service::set_use_automatically`](crate::service::set_use_automatically)) +/// then runs its work locally on this relay-sized runtime, which is correct but +/// slow. Decide routing once at start-up and do not flip it mid-process. pub fn size_threads_for_relaying() { drop(lore_base::runtime::runtime_with_settings(Some( lore_base::runtime::TokioSettings::relay_only(), From 1b0340f2146f7796677a5baad6df3accbbf59606 Mon Sep 17 00:00:00 2001 From: Mattias Jansson Date: Wed, 22 Jul 2026 22:39:15 +0200 Subject: [PATCH 10/20] Document that shutdown does not drain in-flight requests # Summary On shutdown the service awaits only its accept loop, not the connection handlers it spawned, so in-flight requests are aborted rather than drained: a client mid-operation sees the connection close, and a request that was writing is truncated (recovery left to the store's crash-consistency). Document this on service_main; a bounded drain is left as a future improvement. ## Changes - `lore-client/src/cli/commands/service/run.rs`: note the no-drain behaviour on `service_main`. # Test Plan - `cargo build -p lore-client` and `cargo +nightly fmt --all --check`: clean. Signed-off-by: Mattias Jansson --- lore-client/src/cli/commands/service/run.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lore-client/src/cli/commands/service/run.rs b/lore-client/src/cli/commands/service/run.rs index 64d9708d..ff8a90a0 100644 --- a/lore-client/src/cli/commands/service/run.rs +++ b/lore-client/src/cli/commands/service/run.rs @@ -56,6 +56,13 @@ fn detached_working_directory() -> std::path::PathBuf { /// connect to the socket so the blocked `accept` returns and the loop can /// observe it. Waiting on the accept loop rather than on the signal is what /// makes the IPC trigger end the process too. +/// +/// Only the accept loop is awaited, not the per-connection handlers it spawned. +/// In-flight requests are therefore not drained: a client mid-operation sees the +/// connection close and reports it, and a request that was writing is truncated, +/// with recovery left to the store's crash-consistency (the same guarantee a +/// locally interrupted command relies on). A bounded drain of outstanding +/// handlers before exit is a possible future improvement. pub async fn service_main( listening_signal: Option>, ) -> Result<(), ServiceMainError> { From 244362eff8f16e7684a8973ac5baa9e4db92214a Mon Sep 17 00:00:00 2001 From: Mattias Jansson Date: Wed, 22 Jul 2026 22:40:46 +0200 Subject: [PATCH 11/20] Point the closed-connection error at a stopped service MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Summary When the service closes a connection mid-request — which happens when it is stopped or terminated while a request is in flight — the client reported only that the connection closed without a result. Say that the service may have been stopped or terminated, so the cause is clear. ## Changes - `lore/src/remote/call.rs`: extend the closed-without-result error message. # Test Plan - `cargo build -p lore` and `cargo +nightly fmt --all --check`: clean. Signed-off-by: Mattias Jansson --- lore/src/remote/call.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lore/src/remote/call.rs b/lore/src/remote/call.rs index 821ae74e..77658e5c 100644 --- a/lore/src/remote/call.rs +++ b/lore/src/remote/call.rs @@ -124,7 +124,8 @@ pub async fn service_call_impl( } Err(ServiceCallError::internal( - "Lore service closed connection without sending a result", + "Lore service closed the connection without sending a result; \ + it may have been stopped or terminated while the request was in flight", )) } From 9fecd8ac0286fba62c9fb46101fd4658a042f343 Mon Sep 17 00:00:00 2001 From: Mattias Jansson Date: Wed, 22 Jul 2026 22:47:53 +0200 Subject: [PATCH 12/20] Note a service status command as a follow-up # Summary A user cannot query whether the service is running or whether automatic routing is enabled. Record the missing `status` command as a follow-up next to the service subcommands. ## Changes - `lore-client/src/cli/commands/service.rs`: TODO for a `status` command mirroring `shared-store info`. # Test Plan - `cargo build -p lore-client` and `cargo +nightly fmt --all --check`: clean. Signed-off-by: Mattias Jansson --- lore-client/src/cli/commands/service.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lore-client/src/cli/commands/service.rs b/lore-client/src/cli/commands/service.rs index 64c44287..93a53092 100644 --- a/lore-client/src/cli/commands/service.rs +++ b/lore-client/src/cli/commands/service.rs @@ -42,6 +42,9 @@ pub struct ServiceSetUseAutomaticallyArgs { enabled: bool, } +// TODO: add a `status` command reporting whether the service is running +// (process::is_running) and whether use_service_automatically is enabled, +// mirroring the `shared-store info` command. #[derive(Subcommand)] pub enum ServiceCommands { ///Run this process as the service From 7aae4732cd6197a4bbceec314ed6973e92f4e7f7 Mon Sep 17 00:00:00 2001 From: Mattias Jansson Date: Wed, 22 Jul 2026 22:59:44 +0200 Subject: [PATCH 13/20] Remove the unreachable running-as-service guard from start # Summary `start_local` guarded on `running_as_service()`, but that branch was unreachable: `start` runs locally and `ServiceStart` is never sent over IPC, so `start_local` never executes inside the service. It read as a live path and implied a false symmetry with `stop_local`, whose guard is genuinely reached because `ServiceStop` is delivered over IPC. `ensure_running` already no-ops when a service is already running, so removing the guard changes nothing. ## Changes - `lore/src/service.rs`: drop the dead `running_as_service()` branch in `start_local`. # Test Plan - `cargo build -p lore`, `cargo clippy -p lore --all-targets -- -D warnings --no-deps`, `cargo +nightly fmt --all --check`: clean. Signed-off-by: Mattias Jansson --- lore/src/service.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/lore/src/service.rs b/lore/src/service.rs index 507dc0f8..a442bb8d 100644 --- a/lore/src/service.rs +++ b/lore/src/service.rs @@ -56,9 +56,6 @@ async fn start_local( callback: LoreEventCallback, ) -> i32 { let command = async move |_args| -> Result<(), ServiceError> { - if process::running_as_service() { - return Ok(()); - } process::ensure_running() .await .forward::("starting the Lore service")?; From 35bfe8c999b2e03ef3e93221391b63c007c1722f Mon Sep 17 00:00:00 2001 From: Mattias Jansson Date: Wed, 22 Jul 2026 23:04:09 +0200 Subject: [PATCH 14/20] Document the shutdown wake-up dependency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Summary Stopping the service depends on the wake-up connection in `request_shutdown` succeeding. Spell out the consequence of a failure: the polling client path self-corrects, but the termination-signal path discards the error to a detached process's null stderr, so exit is delayed until another connection arrives. The failure is rare — connecting to one's own listening socket essentially only fails under backlog exhaustion or a removed socket file. ## Changes - `lore/src/remote/process.rs`: expand the `request_shutdown` doc comment. # Test Plan - `cargo build -p lore` and `cargo +nightly fmt --all --check`: clean. Signed-off-by: Mattias Jansson --- lore/src/remote/process.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lore/src/remote/process.rs b/lore/src/remote/process.rs index 43c871e6..2deb642e 100644 --- a/lore/src/remote/process.rs +++ b/lore/src/remote/process.rs @@ -45,7 +45,15 @@ pub fn running_as_service() -> bool { /// Stops this process's own service loop. The accept loop is blocked in /// `accept`, so it also needs a connection to wake it before it can observe the /// flag; a failure to make that connection leaves the loop parked, so it is -/// reported rather than ignored. +/// returned rather than ignored. +/// +/// Shutdown therefore depends on this wake-up connection succeeding. A caller +/// that polls afterwards (the client `service stop`, via `wait_until_stopped`) +/// self-corrects, because its probes are themselves connections that wake the +/// loop. The termination-signal path does not poll and discards this error to a +/// detached process's null stderr, so on the rare failure — connecting to one's +/// own listening socket essentially only fails under backlog exhaustion or a +/// removed socket file — exit is delayed until another connection arrives. pub fn request_shutdown() -> Result<(), ServiceProcessError> { let Some(flag) = SHUTDOWN_FLAG.get() else { return Err(ServiceProcessError::internal( From a0d26cf875e710f9d7b47b3811a5dd1370ac8560 Mon Sep 17 00:00:00 2001 From: Mattias Jansson Date: Wed, 22 Jul 2026 23:11:02 +0200 Subject: [PATCH 15/20] Document the single-service-per-process constraint # Summary The shutdown flag is stored in a OnceLock, so only the first service_main in a process registers a flag that request_shutdown can trip. A second in-process service would run an accept loop watching a flag that is never tripped and could not be stopped. Document this on register_shutdown_flag; the CLI runs the service once per process, so only a future embedder or in-process test is constrained. ## Changes - `lore/src/remote/process.rs`: note the single-service-per-process constraint. # Test Plan - `cargo build -p lore` and `cargo +nightly fmt --all --check`: clean. Signed-off-by: Mattias Jansson --- lore/src/remote/process.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lore/src/remote/process.rs b/lore/src/remote/process.rs index 2deb642e..b222e66e 100644 --- a/lore/src/remote/process.rs +++ b/lore/src/remote/process.rs @@ -34,6 +34,12 @@ static SHUTDOWN_FLAG: OnceLock> = OnceLock::new(); /// Called by the service process at startup to publish the flag that stops its /// accept loop, so that a `ServiceStop` arriving over IPC can trip it. +/// +/// Supports a single service per process: the `OnceLock` keeps the first flag, +/// so a second `service_main` in the same process would run an accept loop +/// watching a flag this never trips, and could not be stopped. The CLI runs +/// `service run` once per process, so this only constrains a future embedder or +/// in-process test that starts the service more than once. pub fn register_shutdown_flag(flag: Arc) { let _ = SHUTDOWN_FLAG.set(flag); } From 4f8531a2234a676a3dab04c50cdbcb5d335b65d9 Mon Sep 17 00:00:00 2001 From: Mattias Jansson Date: Wed, 22 Jul 2026 23:17:30 +0200 Subject: [PATCH 16/20] Avoid a probe connection on each service call MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Summary Every service-routed call first called `ensure_running`, whose liveness check opens and drops a connection, then opened a second connection for the real message — two connects, accepts, and handler-task spawns per call, the first a phantom the service reads as an immediate EOF. Send to an existing service first and only start one if that fails, so the common path opens a single connection. ## Changes - `lore/src/remote/call.rs`: serialize the message once, then `connect_and_send` it; on failure, `ensure_running` and retry. Auto-start still happens when no service is listening, and a start refusal still surfaces through the retry. # Test Plan - `uv run pytest scripts/test/test_service.py`: 6 passed, covering the three paths — service already up (single connection), service down (start then retry), and a non-`lore` binary refusing to auto-start. - `cargo clippy -p lore --all-targets -- -D warnings --no-deps` and `cargo +nightly fmt --all --check`: clean. Signed-off-by: Mattias Jansson --- lore/src/remote/call.rs | 59 ++++++++++++++++++++++++----------------- 1 file changed, 35 insertions(+), 24 deletions(-) diff --git a/lore/src/remote/call.rs b/lore/src/remote/call.rs index 77658e5c..1265b106 100644 --- a/lore/src/remote/call.rs +++ b/lore/src/remote/call.rs @@ -70,31 +70,12 @@ pub async fn service_call( }) } -pub async fn service_call_impl( - event_dispatcher: &mut EventDispatcher, - globals: LoreGlobalArgs, - args: ArgsType, -) -> Result { - if !uds_supported() { - return Err(ServiceCallError::internal("OS doesn't support IPC")); - } - - crate::remote::process::ensure_running() - .await - .forward::("starting the Lore service")?; - - let connection = lore_base::lore_spawn_blocking!(|| { +/// Opens a connection to the service and writes the already-serialized message, +/// returning the connection to read the reply from. +async fn connect_and_send(message_bytes: Vec) -> Result { + lore_base::lore_spawn_blocking!(move || { let mut connection = UdsStream::connect().forward::("connecting to local socket")?; - - let message = MessageToServer { - globals, - command: args.to_command(), - }; - - let message_bytes = write_v1_message(message, SerializationType::Json) - .forward::("serializing message")?; - connection .writer() .write_all(&message_bytes) @@ -102,7 +83,37 @@ pub async fn service_call_impl( Ok::(connection) }) .await - .internal("joining connection task")??; + .internal("joining connection task")? +} + +pub async fn service_call_impl( + event_dispatcher: &mut EventDispatcher, + globals: LoreGlobalArgs, + args: ArgsType, +) -> Result { + if !uds_supported() { + return Err(ServiceCallError::internal("OS doesn't support IPC")); + } + + let message = MessageToServer { + globals, + command: args.to_command(), + }; + let message_bytes = write_v1_message(message, SerializationType::Json) + .forward::("serializing message")?; + + // Send to an existing service first, and only start one if that fails, so + // the common path opens a single connection rather than a liveness probe + // followed by the real one. + let connection = match connect_and_send(message_bytes.clone()).await { + Ok(connection) => connection, + Err(_) => { + crate::remote::process::ensure_running() + .await + .forward::("starting the Lore service")?; + connect_and_send(message_bytes).await? + } + }; 'read_from_stream: loop { let mut connection = connection.try_clone().internal("cloning connection")?; From 0851b9b4c10b3b46a6e0203222c9600fd373fbea Mon Sep 17 00:00:00 2001 From: Mattias Jansson Date: Thu, 23 Jul 2026 08:03:02 +0200 Subject: [PATCH 17/20] Test graceful shutdown and concurrent stop # Summary Add the two tractable coverage gaps from the review: that a termination signal stops the service cleanly, and that two stops racing one live service both succeed (the loser's send finds the service gone but must still report success). ## Changes - `scripts/test/test_service.py`: - `test_service_shuts_down_gracefully_on_signal` (SIGTERM and SIGINT, parametrized, POSIX-only): the service exits with code 0. - `test_concurrent_service_stop_all_succeed`: two `service stop` processes race a started service, three rounds, and both exit 0 each round. # Test Plan - `uv run pytest scripts/test/test_service.py`: 9 passed. The two new tests were run three further times and were stable. - `uv run ruff check`: clean. Signed-off-by: Mattias Jansson --- scripts/test/test_service.py | 60 ++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/scripts/test/test_service.py b/scripts/test/test_service.py index 5751bc99..97ae2397 100644 --- a/scripts/test/test_service.py +++ b/scripts/test/test_service.py @@ -4,6 +4,7 @@ import os import platform import shutil +import signal import subprocess import pytest @@ -276,3 +277,62 @@ def test_service_starts_on_demand(new_lore_repo, stopped_service): assert "A " + file_name in map( lambda line: line.strip(" "), repo.status().splitlines() ) + + +@pytest.mark.smoke +@pytest.mark.xdist_group("lore_service") +@pytest.mark.skipif( + platform.system() not in ("Linux", "Darwin"), + reason="POSIX termination signals", +) +@pytest.mark.parametrize("sig", [signal.SIGTERM, signal.SIGINT]) +def test_service_shuts_down_gracefully_on_signal( + lore_service_in_directory, tmp_path, sig +): + """A termination signal stops the service cleanly, exiting with code 0.""" + service_directory = tmp_path / f"service_{sig}" + service_directory.mkdir() + service_process = lore_service_in_directory(service_directory) + + service_process.send_signal(sig) + try: + code = service_process.wait(timeout=10) + except subprocess.TimeoutExpired: + service_process.kill() + pytest.fail(f"the service did not exit on {sig!r}") + assert code == 0, f"the service should exit cleanly on {sig!r}, got {code}" + + +@pytest.mark.smoke +@pytest.mark.xdist_group("lore_service") +@pytest.mark.skipif( + not service_supported(), reason="Service not supported on " + platform.system() +) +def test_concurrent_service_stop_all_succeed( + lore_executable_path, global_dir_name, stopped_service +): + """Two stops racing one live service both report success. + + The one whose send finds the service already gone must still exit 0 rather + than fail on the closed connection. + """ + env = os.environ.copy() + env["LORE_GLOBAL_PATH"] = global_dir_name + + for _ in range(3): + start = run_lore(lore_executable_path, ["service", "start"], global_dir_name) + assert start.returncode == 0, start.stdout + start.stderr + + stops = [ + subprocess.Popen( + [lore_executable_path, "service", "stop"], + env=env, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) + for _ in range(2) + ] + for stop in stops: + out, err = stop.communicate(timeout=15) + assert stop.returncode == 0, f"a concurrent stop failed: {out}{err}" From 0ec50e7dc0c0d8ad0578bf4740b664dc945eae12 Mon Sep 17 00:00:00 2001 From: Mattias Jansson Date: Thu, 23 Jul 2026 09:09:17 +0200 Subject: [PATCH 18/20] Size the runtime lean on the first relayed FFI call MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Summary The CLI sizes its runtime for relaying before running a command, but an embedder calling the FFI has no such step: the first call built the full runtime even when the service is enabled and every call just relays over IPC, so the process kept a full runtime it never uses for its whole lifetime. ## Changes - `lore/src/call_delegation.rs`: the synchronous and asynchronous FFI runners size the runtime for relaying, when the service is enabled, before building it. A no-op once the runtime exists and when the service is off. This covers every FFI entry point uniformly — routed operations relay, and the local ones (`service start`/`stop`/`set-use-automatically`) are lightweight, so the lean runtime suits both. The one case it does not cover: an embedder whose very first call is `set-use-automatically(true)` in the same process, since the setting is not yet on when that call is sized. # Test Plan - `lore/tests/dispatch_runtime.rs` (own test binary, fresh runtime): with the service enabled, the first FFI call builds a two-worker runtime. - `cargo test -p lore --lib` (93), clippy (`-D warnings`), nightly fmt: clean. Signed-off-by: Mattias Jansson --- lore/src/call_delegation.rs | 16 +++++++++++++++ lore/tests/dispatch_runtime.rs | 37 ++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) create mode 100644 lore/tests/dispatch_runtime.rs diff --git a/lore/src/call_delegation.rs b/lore/src/call_delegation.rs index cb7e6443..15eb1e4e 100644 --- a/lore/src/call_delegation.rs +++ b/lore/src/call_delegation.rs @@ -106,6 +106,20 @@ pub fn will_use_service() -> bool { enabled } +/// Sizes the shared runtime before an FFI entry point first builds it: lean when +/// the call will be relayed to the service, so an embedder whose first call is a +/// routed API does not build the full runtime it never uses. A no-op once the +/// runtime exists, and when the service is not in use. The routed operations all +/// relay and the local ones (`service start`/`stop`/`set-use-automatically`) are +/// lightweight, so the lean runtime suits both. +fn size_runtime_for_dispatch() { + if will_use_service() { + drop(lore_base::runtime::runtime_with_settings(Some( + lore_base::runtime::TokioSettings::relay_only(), + ))); + } +} + pub(crate) fn run_synchronously< ArgsType: InvokableLoreArgs + Clone + Send + 'static, Handler: Fn(LoreGlobalArgs, ArgsType, LoreEventCallback) -> Fut, @@ -116,6 +130,7 @@ pub(crate) fn run_synchronously< callback: LoreEventCallbackConfig, handler: Handler, ) -> i32 { + size_runtime_for_dispatch(); let callback = lore_revision::event::convert_event_callback(callback); let globals = globals.clone(); let args = args.clone(); @@ -132,6 +147,7 @@ pub(crate) fn run_asynchronously< callback: LoreEventCallbackConfig, handler: Handler, ) { + size_runtime_for_dispatch(); let callback = lore_revision::event::convert_event_callback(callback); let globals = globals.clone(); let args = args.clone(); diff --git a/lore/tests/dispatch_runtime.rs b/lore/tests/dispatch_runtime.rs new file mode 100644 index 00000000..788251d4 --- /dev/null +++ b/lore/tests/dispatch_runtime.rs @@ -0,0 +1,37 @@ +// SPDX-FileCopyrightText: 2026 Epic Games, Inc. +// SPDX-License-Identifier: MIT + +//! An embedder's first FFI call, when the service is enabled, must size the +//! shared runtime for relaying rather than build the full runtime it never +//! uses. This runs in its own test binary so the process-wide runtime is built +//! fresh here. + +#[test] +fn first_ffi_call_sizes_runtime_for_relaying_when_service_enabled() { + let global = std::env::temp_dir().join("lore_dispatch_runtime_test_global"); + // Safety: set before any runtime use, and this test binary is + // single-threaded at this point. + unsafe { + std::env::set_var("LORE_USE_SERVICE", "1"); + std::env::set_var("LORE_GLOBAL_PATH", &global); + std::env::remove_var("LORE_WORKER_THREADS"); + } + + // Any FFI call goes through the synchronous runner, which sizes the runtime + // before building it. `set-use-automatically` only writes the global config + // — here the isolated one under LORE_GLOBAL_PATH — so it touches no socket + // and starts no service. + let globals = lore::interface::LoreGlobalArgs::default(); + let args = lore::interface::LoreServiceSetUseAutomaticallyArgs { enabled: 1 }; + let callback = lore::interface::LoreEventCallbackConfig { + user_context: 0, + func: None, + }; + lore::interface::lore_service_set_use_automatically(&globals, &args, callback); + + assert_eq!( + lore::runtime().metrics().num_workers(), + 2, + "the first FFI call must size the runtime lean when the service is enabled" + ); +} From 311599cb015597dc5b8f73599f9a62388c94a6dc Mon Sep 17 00:00:00 2001 From: Mattias Jansson Date: Thu, 23 Jul 2026 20:26:29 +0200 Subject: [PATCH 19/20] Fix pedantic clippy lints and doc trailing newline # Summary The pedantic lints enabled in .cargo/config.toml flagged two constructs, and the regenerated CLI command reference had a double trailing newline the end-of-file-fixer objects to. ## Changes - `lore/src/call_delegation.rs`: `Result::map(..).unwrap_or(false)` becomes `is_ok_and(..)` (`clippy::map_unwrap_or`). - `lore/src/remote/call.rs`: the single-arm `match` becomes `if let ... else` (`clippy::single_match_else`). - `docs/reference/lore-cli-commands.md`: collapse the trailing newline to one. # Test Plan - `cargo clippy --all-targets -- -D warnings --no-deps`: clean across the workspace. - `cargo +nightly fmt --all --check` and `cargo test -p lore --lib` (93): clean. Signed-off-by: Mattias Jansson --- docs/reference/lore-cli-commands.md | 1 - lore/src/call_delegation.rs | 3 +-- lore/src/remote/call.rs | 15 +++++++-------- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/docs/reference/lore-cli-commands.md b/docs/reference/lore-cli-commands.md index 44176dbd..d83ba52c 100644 --- a/docs/reference/lore-cli-commands.md +++ b/docs/reference/lore-cli-commands.md @@ -2742,4 +2742,3 @@ Manage the shared store This document was generated automatically by clap-markdown. - diff --git a/lore/src/call_delegation.rs b/lore/src/call_delegation.rs index 15eb1e4e..423bcd41 100644 --- a/lore/src/call_delegation.rs +++ b/lore/src/call_delegation.rs @@ -76,8 +76,7 @@ pub(crate) async fn use_service() -> bool { } let enabled = GlobalConfig::load() .await - .map(|config| config.use_service_automatically()) - .unwrap_or(false); + .is_ok_and(|config| config.use_service_automatically()); *USE_SERVICE.write() = Some(enabled); enabled } diff --git a/lore/src/remote/call.rs b/lore/src/remote/call.rs index 1265b106..fbfac779 100644 --- a/lore/src/remote/call.rs +++ b/lore/src/remote/call.rs @@ -105,14 +105,13 @@ pub async fn service_call_impl( // Send to an existing service first, and only start one if that fails, so // the common path opens a single connection rather than a liveness probe // followed by the real one. - let connection = match connect_and_send(message_bytes.clone()).await { - Ok(connection) => connection, - Err(_) => { - crate::remote::process::ensure_running() - .await - .forward::("starting the Lore service")?; - connect_and_send(message_bytes).await? - } + let connection = if let Ok(connection) = connect_and_send(message_bytes.clone()).await { + connection + } else { + crate::remote::process::ensure_running() + .await + .forward::("starting the Lore service")?; + connect_and_send(message_bytes).await? }; 'read_from_stream: loop { From 332e9736f9fdd0743865d640193d0f51cd333002 Mon Sep 17 00:00:00 2001 From: Mattias Jansson Date: Thu, 23 Jul 2026 21:24:54 +0200 Subject: [PATCH 20/20] Log a real command name for the service commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Summary The service handlers passed a string literal as the `caller` argument to `no_repository_call`, and the command logging derives the command name from `type_name` of that argument — so the debug log read `Executing command: &str` instead of a command path. Pass the handler function, as the other handlers do, so the name resolves properly. ## Changes - `lore/src/service.rs`: pass `start` / `stop` / `set_use_automatically` rather than string literals to `no_repository_call`. # Test Plan - `lore --debug service set-use-automatically true` now logs `Executing command: lore::service::set_use_automatically`, matching the form other commands use (e.g. `lore::shared_store::info`). - `cargo clippy --all-targets -- -D warnings --no-deps`, `cargo +nightly fmt --all --check`, `cargo test -p lore --lib` (93): clean. Signed-off-by: Mattias Jansson --- lore/src/service.rs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/lore/src/service.rs b/lore/src/service.rs index a442bb8d..9be4010f 100644 --- a/lore/src/service.rs +++ b/lore/src/service.rs @@ -61,7 +61,7 @@ async fn start_local( .forward::("starting the Lore service")?; Ok(()) }; - no_repository_call(globals, callback, args, "service start", command).await + no_repository_call(globals, callback, args, start, command).await } #[repr(C)] @@ -129,7 +129,7 @@ async fn stop_local( } Ok(()) }; - no_repository_call(globals, callback, args, "service stop", command).await + no_repository_call(globals, callback, args, stop, command).await } #[repr(C)] @@ -185,12 +185,5 @@ async fn set_use_automatically_local( invalidate_use_service_cache(); Ok(()) }; - no_repository_call( - globals, - callback, - args, - "service set-use-automatically", - command, - ) - .await + no_repository_call(globals, callback, args, set_use_automatically, command).await }