From ead2ca53b8c7cd0eb25d911f0ddb13bd0b485245 Mon Sep 17 00:00:00 2001 From: Scott Werner Date: Mon, 28 Sep 2026 14:30:36 -0400 Subject: [PATCH] Keep local artifacts under the storage directory Servers have always moved a local artifact store under the storage directory at startup, whatever `local.root` said. Browser-wizard installs write `local.root = "/objects"`, so honoring that root would move their store and hide every artifact already written, with nothing to migrate it. Restore the storage-directory override for local roots and leave honoring custom roots to a change that migrates existing objects. Installer metadata still goes through the override, so it lands where the server reads. Co-Authored-By: Claude Opus 5.5 --- .../administration/server-configuration.mdx | 4 ---- .../fabro-cli/tests/it/scenario/artifacts.rs | 23 +++++++------------ lib/apps/fabro-server/src/serve.rs | 16 +++++++++---- lib/foundation/fabro-types/src/dense.rs | 11 ++++----- 4 files changed, 24 insertions(+), 30 deletions(-) diff --git a/docs/public/administration/server-configuration.mdx b/docs/public/administration/server-configuration.mdx index d6850d3d2..48315c0e6 100644 --- a/docs/public/administration/server-configuration.mdx +++ b/docs/public/administration/server-configuration.mdx @@ -247,10 +247,6 @@ prefix = "artifacts" root = "/var/lib/fabro/objects" ``` -A custom local artifact root is preserved at startup and when `--storage-dir` -changes the server's storage directory. When `local.root` is omitted, the -default `/objects/artifacts` location follows that override. - The wizard only covers AWS S3 bucket/region plus one of: - runtime credentials already supplied by the deployment environment diff --git a/lib/apps/fabro-cli/tests/it/scenario/artifacts.rs b/lib/apps/fabro-cli/tests/it/scenario/artifacts.rs index 952a99621..81f327746 100644 --- a/lib/apps/fabro-cli/tests/it/scenario/artifacts.rs +++ b/lib/apps/fabro-cli/tests/it/scenario/artifacts.rs @@ -16,14 +16,14 @@ async fn artifact_worker_captures_large_files_in_the_configured_local_store() { return; } let context = test_context!(); - let objects = context.temp_dir.join("selected-artifact-root"); - let settings = format!( - "\n[server.artifacts]\nprovider = \"local\"\nprefix = \"selected-prefix\"\n[server.artifacts.local]\nroot = {:?}\n", - objects.to_str().unwrap() - ); - // RunningServer also passes --storage-dir. The explicit artifact directory - // must still receive the captures, independently of the database root. - let server = RunningServer::start_with(&settings, &[]).await; + let server = RunningServer::start_with( + "\n[server.artifacts]\nprovider = \"local\"\nprefix = \"selected-prefix\"\n", + &[], + ) + .await; + // RunningServer explicitly selects --storage-dir, which also selects the + // local artifact root. Inspect that resolved backend, outside the sandbox. + let objects = server.storage_dir.join("objects/artifacts"); let workspace = artifact_workspace(&context); tokio::fs::write(workspace.join("workflow.fabro"), r#"digraph Capture { graph [goal="Capture binary files", default_max_retries=0] @@ -58,13 +58,6 @@ async fn artifact_worker_captures_large_files_in_the_configured_local_store() { serde_json::to_value(rebuilt.unwrap().artifacts).unwrap(), projection["artifacts"] ); - assert!( - !server - .storage_dir - .join(format!("objects/artifacts/selected-prefix/{run_id}")) - .exists(), - "captures must not be redirected to the default artifact directory" - ); for (path, size) in [ ("medium.bin", 3 * 1024 * 1024), diff --git a/lib/apps/fabro-server/src/serve.rs b/lib/apps/fabro-server/src/serve.rs index 57f2c15b2..deedd584f 100644 --- a/lib/apps/fabro-server/src/serve.rs +++ b/lib/apps/fabro-server/src/serve.rs @@ -1348,16 +1348,18 @@ mod tests { } #[test] - fn runtime_server_settings_preserve_custom_artifact_root() { + fn runtime_server_settings_keep_local_artifacts_under_storage_dir() { + // Servers have always kept local artifacts under the storage + // directory, including installs whose settings name a `local.root`. let settings = server_settings( r#" _version = 1 [server.storage] root = "/srv/from-disk" [server.artifacts] -prefix = "selected-prefix" +prefix = "artifacts" [server.artifacts.local] -root = "/mnt/artifact-files" +root = "/srv/from-disk/objects" "#, ); for storage_root in ["/srv/from-disk", "/srv/from-runtime"] { @@ -1365,7 +1367,13 @@ root = "/mnt/artifact-files" .clone() .with_storage_override(Path::new(storage_root)); assert_eq!(resolved.server.storage.root, storage_root); - assert_eq!(resolved.server.artifacts, settings.server.artifacts); + assert_eq!(resolved.server.artifacts.prefix, "artifacts"); + assert_eq!( + resolved.server.artifacts.store, + fabro_types::settings::ObjectStoreSettings::Local { + root: format!("{storage_root}/objects/artifacts"), + } + ); // Startup and config reload can apply the same override again. assert_eq!( resolved diff --git a/lib/foundation/fabro-types/src/dense.rs b/lib/foundation/fabro-types/src/dense.rs index 3fe7e5cd2..97ee640aa 100644 --- a/lib/foundation/fabro-types/src/dense.rs +++ b/lib/foundation/fabro-types/src/dense.rs @@ -16,14 +16,11 @@ pub struct ServerSettings { impl ServerSettings { #[must_use] pub fn with_storage_override(mut self, path: &Path) -> Self { - // Only the derived default follows the storage directory. A custom - // artifact location is independent of the database and runtime root. - let default_artifact_root = - ServerArtifactsSettings::default_local_root(Path::new(&self.server.storage.root)); + // The local artifact store always lives under the storage directory. + // A configured `local.root` has never moved it, and existing objects + // are only found there. if let ObjectStoreSettings::Local { root } = &mut self.server.artifacts.store { - if *root == default_artifact_root { - *root = ServerArtifactsSettings::default_local_root(path); - } + *root = ServerArtifactsSettings::default_local_root(path); } self.server.storage.root = path.display().to_string(); self