From 5b1c40764d6ae3f54155fc9890e39512616efa1d Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Tue, 21 Apr 2026 23:37:15 -0400 Subject: [PATCH 1/2] refactor(cli): enforce CLI/server settings boundary Move server-only settings reads out of user-facing CLI commands into a dedicated local_server module, the install/uninstall exceptions, and the worker subcommand. Adds bin/dev/check-boundary.sh to prevent regressions. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/rust.yml | 13 ++ bin/dev/check-boundary.sh | 86 ++++++++++ lib/crates/fabro-cli/src/commands/doctor.rs | 160 +----------------- lib/crates/fabro-cli/src/commands/install.rs | 46 +++-- .../fabro-cli/src/commands/pr/create.rs | 6 + lib/crates/fabro-cli/src/commands/pr/mod.rs | 8 + .../fabro-cli/src/commands/run/command.rs | 1 - .../fabro-cli/src/commands/run/create.rs | 19 +-- .../fabro-cli/src/commands/run/output.rs | 7 +- .../fabro-cli/src/commands/run/resume.rs | 10 +- .../src/commands/server/foreground.rs | 4 +- .../fabro-cli/src/commands/server/mod.rs | 21 +-- .../fabro-cli/src/commands/server/record.rs | 8 +- .../fabro-cli/src/commands/server/start.rs | 53 +++--- .../fabro-cli/src/commands/uninstall.rs | 16 +- lib/crates/fabro-cli/src/local_server.rs | 65 +++++++ lib/crates/fabro-cli/src/main.rs | 17 +- lib/crates/fabro-cli/src/server_client.rs | 21 ++- lib/crates/fabro-cli/src/user_config.rs | 26 +-- lib/crates/fabro-cli/tests/it/cmd/install.rs | 2 +- lib/crates/fabro-cli/tests/it/cmd/run.rs | 8 +- lib/crates/fabro-cli/tests/it/cmd/support.rs | 6 +- .../tests/it/workflow/dry_run_examples.rs | 5 - lib/crates/fabro-config/src/lib.rs | 2 +- lib/crates/fabro-config/src/storage.rs | 39 ++--- lib/crates/fabro-install/src/lib.rs | 4 +- lib/crates/fabro-server/src/diagnostics.rs | 77 ++++++++- lib/crates/fabro-server/src/install.rs | 2 +- lib/crates/fabro-server/src/serve.rs | 2 +- lib/crates/fabro-server/src/server.rs | 4 +- .../fabro-server/tests/it/api/install.rs | 12 +- lib/crates/fabro-server/tests/it/api/tcp.rs | 4 +- 32 files changed, 414 insertions(+), 340 deletions(-) create mode 100755 bin/dev/check-boundary.sh create mode 100644 lib/crates/fabro-cli/src/local_server.rs diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 76e5e9861..ac96ed440 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -10,6 +10,7 @@ on: - "Cargo.lock" - ".cargo/**" - ".config/**" + - "bin/dev/**" - "openapi/**" - ".github/workflows/rust.yml" pull_request: @@ -21,6 +22,7 @@ on: - "Cargo.lock" - ".cargo/**" - ".config/**" + - "bin/dev/**" - "openapi/**" - ".github/workflows/rust.yml" workflow_dispatch: @@ -35,6 +37,17 @@ env: CARGO_TERM_COLOR: always jobs: + boundary: + name: Boundary + runs-on: ubuntu-24.04-x86-32-cores + permissions: + contents: read + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + persist-credentials: false + - run: bin/dev/check-boundary.sh + fmt: name: Format runs-on: ubuntu-24.04-x86-32-cores diff --git a/bin/dev/check-boundary.sh b/bin/dev/check-boundary.sh new file mode 100755 index 000000000..6842a5aea --- /dev/null +++ b/bin/dev/check-boundary.sh @@ -0,0 +1,86 @@ +#!/usr/bin/env bash +set -euo pipefail + +cd "$(dirname "$0")/../.." + +symbol_allowlist=( + "lib/crates/fabro-cli/src/local_server.rs" + "lib/crates/fabro-cli/src/commands/install.rs" + "lib/crates/fabro-cli/src/commands/uninstall.rs" + "lib/crates/fabro-cli/src/commands/run/runner.rs" + "lib/crates/fabro-cli/src/commands/pr/mod.rs" + "lib/crates/fabro-cli/src/commands/pr/create.rs" +) + +deprecated_helper_allowlist=( + "lib/crates/fabro-cli/src/user_config.rs" + "lib/crates/fabro-cli/src/commands/pr/mod.rs" + "lib/crates/fabro-cli/src/commands/pr/create.rs" +) + +temporary_exemptions=( + "lib/crates/fabro-cli/src/commands/pr/mod.rs" + "lib/crates/fabro-cli/src/commands/pr/create.rs" +) + +exemption_marker="boundary-exempt(pr-api): remove with follow-up #1" + +in_array() { + local needle=$1 + shift + local item + for item in "$@"; do + if [[ "$item" == "$needle" ]]; then + return 0 + fi + done + return 1 +} + +find_matches() { + local pattern=$1 + if command -v rg >/dev/null 2>&1; then + rg -l "$pattern" lib/crates/fabro-cli/src --glob '*.rs' || true + else + grep -R -l -E "$pattern" lib/crates/fabro-cli/src --include='*.rs' || true + fi +} + +fail=0 + +while IFS= read -r path; do + [[ -z "$path" ]] && continue + if ! in_array "$path" "${symbol_allowlist[@]}"; then + echo "boundary check failed: gated server symbol used outside allowlist: $path" >&2 + fail=1 + fi +done < <(find_matches 'fabro_config::resolve_server_from_file|fabro_config::resolve_server\b|Storage::new') + +while IFS= read -r path; do + [[ -z "$path" ]] && continue + if ! in_array "$path" "${deprecated_helper_allowlist[@]}"; then + echo "boundary check failed: deprecated user_config::storage_dir used outside allowlist: $path" >&2 + fail=1 + fi +done < <(find_matches 'user_config::storage_dir') + +for path in "${temporary_exemptions[@]}"; do + if ! grep -q "$exemption_marker" "$path"; then + echo "boundary check failed: missing temporary exemption marker in $path" >&2 + fail=1 + fi +done + +while IFS= read -r path; do + [[ -z "$path" ]] && continue + if ! in_array "$path" "${temporary_exemptions[@]}"; then + echo "boundary check failed: unexpected temporary exemption marker in $path" >&2 + fail=1 + fi +done < <(find_matches "$exemption_marker") + +if [[ $fail -ne 0 ]]; then + exit 1 +fi + +echo "CLI/server boundary checks passed." diff --git a/lib/crates/fabro-cli/src/commands/doctor.rs b/lib/crates/fabro-cli/src/commands/doctor.rs index 2a5fc1ed5..cdf9931ac 100644 --- a/lib/crates/fabro-cli/src/commands/doctor.rs +++ b/lib/crates/fabro-cli/src/commands/doctor.rs @@ -1,9 +1,4 @@ -#![expect( - clippy::disallowed_methods, - reason = "CLI `doctor` command: sync directory scan in command handler" -)] - -use std::path::{Path, PathBuf}; +use std::path::PathBuf; use anyhow::Result; use fabro_api::types as api_types; @@ -21,7 +16,6 @@ use fabro_util::version::FABRO_VERSION; use crate::args::DoctorArgs; use crate::command_context::CommandContext; use crate::shared::print_json_pretty; -use crate::user_config; pub(crate) fn check_config(settings_path: Option) -> CheckResult { match settings_path { @@ -50,65 +44,6 @@ pub(crate) fn check_config(settings_path: Option) -> CheckResult { } } -#[derive(Debug, Clone, PartialEq, Eq)] -struct StorageDirStatus { - path: PathBuf, - exists: bool, - readable: bool, - writable: bool, -} - -fn probe_storage_dir(path: &Path) -> StorageDirStatus { - let exists = path.is_dir(); - let readable = exists && std::fs::read_dir(path).is_ok(); - let writable = exists && tempfile::tempfile_in(path).is_ok(); - - StorageDirStatus { - path: path.to_path_buf(), - exists, - readable, - writable, - } -} - -fn check_storage_dir(status: &StorageDirStatus) -> CheckResult { - let display = contract_tilde(&status.path); - let display = display.display(); - let details = vec![ - CheckDetail::new(format!( - "Exists: {}", - if status.exists { "yes" } else { "no" } - )), - CheckDetail::new(format!( - "Readable: {}", - if status.readable { "yes" } else { "no" } - )), - CheckDetail::new(format!( - "Writable: {}", - if status.writable { "yes" } else { "no" } - )), - ]; - let is_healthy = status.exists && status.readable && status.writable; - - CheckResult { - name: "Storage directory".to_string(), - status: if is_healthy { - CheckStatus::Pass - } else { - CheckStatus::Error - }, - summary: display.to_string(), - details, - remediation: if is_healthy { - None - } else if !status.exists { - Some(format!("Create the directory: mkdir -p {display}")) - } else { - Some(format!("Fix permissions on {display}")) - }, - } -} - fn check_version_parity(server_version: &str) -> CheckResult { let cli_version = FABRO_VERSION; if server_version == cli_version { @@ -230,19 +165,11 @@ pub(crate) async fn run_doctor( let settings_config_path = active_settings_path(None); - let settings = user_config::load_settings().unwrap_or_default(); - let storage_dir_path = - user_config::storage_dir(&settings).unwrap_or_else(|_| user_config::default_storage_dir()); - let storage_dir = probe_storage_dir(&storage_dir_path); - - let local_checks = vec![ - check_config( - settings_config_path - .exists() - .then_some(settings_config_path), - ), - check_storage_dir(&storage_dir), - ]; + let local_checks = vec![check_config( + settings_config_path + .exists() + .then_some(settings_config_path), + )]; let mut report = CheckReport { title: "Fabro Doctor".to_string(), @@ -409,81 +336,6 @@ mod tests { assert!(result.remediation.is_some()); } - // -- check_storage_dir -- - - #[test] - fn probe_storage_dir_existing_dir_is_readable_and_writable() { - let dir = tempfile::tempdir().unwrap(); - let status = probe_storage_dir(dir.path()); - - assert_eq!(status, StorageDirStatus { - path: dir.path().to_path_buf(), - exists: true, - readable: true, - writable: true, - }); - } - - #[test] - fn probe_storage_dir_missing_dir_is_not_usable() { - let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("missing"); - let status = probe_storage_dir(&path); - - assert_eq!(status, StorageDirStatus { - path, - exists: false, - readable: false, - writable: false, - }); - } - - #[test] - fn check_storage_dir_pass() { - let result = check_storage_dir(&StorageDirStatus { - path: PathBuf::from("/home/user/.fabro"), - exists: true, - readable: true, - writable: true, - }); - - assert_eq!(result.status, CheckStatus::Pass); - assert_eq!(result.summary, "/home/user/.fabro"); - assert!(result.remediation.is_none()); - assert_eq!(result.details.len(), 3); - } - - #[test] - fn check_storage_dir_not_exists() { - let result = check_storage_dir(&StorageDirStatus { - path: PathBuf::from("/tmp/nonexistent-fabro-doctor-test-xyz"), - exists: false, - readable: false, - writable: false, - }); - - assert_eq!(result.status, CheckStatus::Error); - assert!(result.summary.contains("nonexistent-fabro-doctor-test-xyz")); - assert!(result.remediation.as_deref().unwrap().contains("mkdir -p")); - } - - #[test] - fn check_storage_dir_not_writable() { - let result = check_storage_dir(&StorageDirStatus { - path: PathBuf::from("/home/user/.fabro"), - exists: true, - readable: true, - writable: false, - }); - - assert_eq!(result.status, CheckStatus::Error); - assert_eq!(result.summary, "/home/user/.fabro"); - assert_eq!( - result.remediation.as_deref(), - Some("Fix permissions on /home/user/.fabro") - ); - } - #[test] fn check_version_parity_warns_on_mismatch() { let result = check_version_parity("0.0.0-test"); diff --git a/lib/crates/fabro-cli/src/commands/install.rs b/lib/crates/fabro-cli/src/commands/install.rs index 010078cae..18d9e43fd 100644 --- a/lib/crates/fabro-cli/src/commands/install.rs +++ b/lib/crates/fabro-cli/src/commands/install.rs @@ -5,7 +5,7 @@ use std::future::Future; use std::net::SocketAddr; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::process::Stdio; use std::time::Duration; @@ -1161,7 +1161,7 @@ fn persist_server_env_secrets(storage_dir: &Path, secrets: &[(String, String)]) return Ok(()); } - let env_path = Storage::new(storage_dir).server_state().env_path(); + let env_path = Storage::new(storage_dir).runtime_state().env_path(); envfile::merge_env_file(&env_path, secrets.iter().cloned()) .with_context(|| format!("merging server env secrets into {}", env_path.display()))?; Ok(()) @@ -1223,7 +1223,7 @@ fn persist_github_install_changes( writes: &PendingGitHubInstallWrite<'_>, ) -> Result<()> { let storage = Storage::new(storage_dir); - let server_env_path = storage.server_state().env_path(); + let server_env_path = storage.runtime_state().env_path(); let vault_path = storage.secrets_path(); let previous_server_env = std::fs::read_to_string(&server_env_path).ok(); let previous_vault = std::fs::read_to_string(&vault_path).ok(); @@ -1292,6 +1292,22 @@ fn render_server_resolve_errors(errors: Vec) -> anyhow::Error { ) } +fn resolved_server_storage_dir(settings: &SettingsLayer) -> Result { + let resolved = + fabro_config::resolve_server_from_file(settings).map_err(render_server_resolve_errors)?; + let resolved_root = resolved + .storage + .root + .resolve(|name| std::env::var(name).ok()) + .map_err(|err| { + anyhow::anyhow!( + "failed to resolve {}: {err}", + resolved.storage.root.as_source() + ) + })?; + Ok(PathBuf::from(resolved_root.value)) +} + async fn write_artifact_store_metadata( settings: &SettingsLayer, fabro_version: &str, @@ -1494,7 +1510,9 @@ async fn run_install_github_inner( .context("failed to parse existing settings.toml")?, args.storage_dir.as_deref(), ); - let storage_dir = user_config::storage_dir(&parsed_settings).unwrap_or_else(|_| { + // Install is a documented server-host exception: it may read [server.*] + // directly. + let storage_dir = resolved_server_storage_dir(&parsed_settings).unwrap_or_else(|_| { args.storage_dir .clone_path() .unwrap_or_else(default_storage_dir) @@ -1586,7 +1604,7 @@ async fn run_install_github_inner( .and_then(|auth| auth.methods) .unwrap_or_default(); let token = dev_token::read_dev_token_file( - &Storage::new(&storage_dir).server_state().dev_token_path(), + &Storage::new(&storage_dir).runtime_state().dev_token_path(), ); print_auth_status(&methods, token.as_deref(), &s, printer); fabro_util::printerr!(printer, ""); @@ -1641,7 +1659,9 @@ async fn run_install_inner( let s = Styles::detect_stderr(); let emoji = console::Emoji("⚒️ ", ""); let cli_settings = user_config::load_settings_with_storage_dir(args.storage_dir.as_deref())?; - let storage_dir = user_config::storage_dir(&cli_settings)?; + // Install is a documented server-host exception: it may read [server.*] + // directly. + let storage_dir = resolved_server_storage_dir(&cli_settings)?; let server_was_running = record::active_server_record(&storage_dir)?.is_some(); let fabro_dir = fabro_util::Home::from_env().root().to_path_buf(); let config_path = fabro_dir.join(SETTINGS_CONFIG_FILENAME); @@ -1823,7 +1843,7 @@ async fn run_install_inner( &fabro_util::Home::from_env().dev_token_path(), )?; dev_token::write_dev_token( - &Storage::new(&storage_dir).server_state().dev_token_path(), + &Storage::new(&storage_dir).runtime_state().dev_token_path(), &token, )?; fabro_util::printerr!( @@ -1879,7 +1899,7 @@ async fn run_install_inner( " {} Saved {} runtime secrets to {}", s.green.apply_to("✔"), server_env_pairs.len(), - path::contract_tilde(&Storage::new(&storage_dir).server_state().env_path()).display() + path::contract_tilde(&Storage::new(&storage_dir).runtime_state().env_path()).display() ); fabro_util::printerr!( printer, @@ -1911,7 +1931,7 @@ async fn run_install_inner( .map(Vec::as_slice) .unwrap_or_default(); let token = dev_token::read_dev_token_file( - &Storage::new(&storage_dir).server_state().dev_token_path(), + &Storage::new(&storage_dir).runtime_state().dev_token_path(), ); print_auth_status(methods, token.as_deref(), &s, printer); fabro_util::printerr!(printer, ""); @@ -2561,7 +2581,7 @@ client_id = "client-id" .unwrap(); let server_env = - std::fs::read_to_string(Storage::new(dir.path()).server_state().env_path()).unwrap(); + std::fs::read_to_string(Storage::new(dir.path()).runtime_state().env_path()).unwrap(); assert!(server_env.contains("SESSION_SECRET=session")); assert!(server_env.contains("FABRO_JWT_PUBLIC_KEY=public-key")); assert_eq!(created.calls_async().await, 2); @@ -2817,7 +2837,7 @@ client_id = "client-id" .await; assert!(result.is_err()); - assert!(Storage::new(dir.path()).server_state().env_path().exists()); + assert!(Storage::new(dir.path()).runtime_state().env_path().exists()); assert!(!settings_path.exists()); assert!(stop_called.load(Ordering::SeqCst)); } @@ -2861,7 +2881,7 @@ client_id = "client-id" fn persist_github_install_changes_replaces_app_env_keys_with_token_secret() { let dir = tempfile::tempdir().unwrap(); let storage = Storage::new(dir.path()); - let server_env_path = storage.server_state().env_path(); + let server_env_path = storage.runtime_state().env_path(); envfile::write_env_file( &server_env_path, &std::collections::HashMap::from([ @@ -2923,7 +2943,7 @@ client_id = "client-id" fn persist_github_install_changes_replaces_token_secret_with_app_env_keys() { let dir = tempfile::tempdir().unwrap(); let storage = Storage::new(dir.path()); - let server_env_path = storage.server_state().env_path(); + let server_env_path = storage.runtime_state().env_path(); envfile::write_env_file( &server_env_path, &std::collections::HashMap::from([("KEEP_ME".to_string(), "1".to_string())]), diff --git a/lib/crates/fabro-cli/src/commands/pr/create.rs b/lib/crates/fabro-cli/src/commands/pr/create.rs index b0d10a56e..78d9703ad 100644 --- a/lib/crates/fabro-cli/src/commands/pr/create.rs +++ b/lib/crates/fabro-cli/src/commands/pr/create.rs @@ -21,6 +21,10 @@ use crate::shared::print_json_pretty; use crate::shared::repo::ensure_matching_repo_origin; use crate::user_config; +#[allow( + deprecated, + reason = "boundary-exempt(pr-api): remove with follow-up #1 when PR ops move server-side" +)] pub(super) async fn create_command( args: PrCreateArgs, cli: &CliSettings, @@ -101,6 +105,8 @@ pub(super) async fn create_command( ); } + // boundary-exempt(pr-api): remove with follow-up #1 when PR ops move + // server-side. let vault = user_config::storage_dir(ctx.machine_settings()) .ok() .and_then(|dir| Vault::load(Storage::new(&dir).secrets_path()).ok()) diff --git a/lib/crates/fabro-cli/src/commands/pr/mod.rs b/lib/crates/fabro-cli/src/commands/pr/mod.rs index 5b1169f4a..84b6a0b57 100644 --- a/lib/crates/fabro-cli/src/commands/pr/mod.rs +++ b/lib/crates/fabro-cli/src/commands/pr/mod.rs @@ -37,12 +37,18 @@ pub(crate) async fn dispatch( } } +#[allow( + deprecated, + reason = "boundary-exempt(pr-api): remove with follow-up #1 when PR ops move server-side" +)] fn load_github_credentials_required( cli: &CliSettings, cli_layer: &CliLayer, printer: Printer, ) -> Result { let ctx = CommandContext::base(printer, cli.clone(), cli_layer)?; + // boundary-exempt(pr-api): remove with follow-up #1 when PR ops move + // server-side. let server_settings = fabro_config::resolve_server_from_file(ctx.machine_settings()).map_err(|errors| { anyhow!( @@ -54,6 +60,8 @@ fn load_github_credentials_required( .join("\n") ) })?; + // boundary-exempt(pr-api): remove with follow-up #1 when PR ops move + // server-side. let vault = user_config::storage_dir(ctx.machine_settings()) .ok() .and_then(|dir| fabro_vault::Vault::load(Storage::new(&dir).secrets_path()).ok()); diff --git a/lib/crates/fabro-cli/src/commands/run/command.rs b/lib/crates/fabro-cli/src/commands/run/command.rs index b2866b4be..ffb83ce03 100644 --- a/lib/crates/fabro-cli/src/commands/run/command.rs +++ b/lib/crates/fabro-cli/src/commands/run/command.rs @@ -71,7 +71,6 @@ pub(crate) async fn execute( super::output::print_run_summary_with_client( &client, &created_run.run_id, - created_run.local_run_dir.as_deref(), styles, printer, ) diff --git a/lib/crates/fabro-cli/src/commands/run/create.rs b/lib/crates/fabro-cli/src/commands/run/create.rs index abc9b14a7..4c34ef861 100644 --- a/lib/crates/fabro-cli/src/commands/run/create.rs +++ b/lib/crates/fabro-cli/src/commands/run/create.rs @@ -1,6 +1,3 @@ -use std::path::PathBuf; - -use fabro_config::Storage; use fabro_config::load::load_settings_user; use fabro_config::user::active_settings_path; use fabro_types::RunId; @@ -13,11 +10,9 @@ use super::overrides::run_args_layer; use crate::args::RunArgs; use crate::command_context::CommandContext; use crate::manifest_builder::{ManifestBuildInput, build_run_manifest, run_manifest_args}; -use crate::user_config; pub(crate) struct CreatedRun { - pub(crate) run_id: RunId, - pub(crate) local_run_dir: Option, + pub(crate) run_id: RunId, } /// Create a workflow run: allocate run directory, persist RunSpec, return @@ -54,7 +49,6 @@ pub(crate) async fn create_run( user_layer: load_settings_user()?, user_settings_path: Some(active_settings_path(None)), })?; - let target = user_config::resolve_server_target(&args.target, ctx.machine_settings())?; let client = ctx.server().await?; if !quiet { let preflight = client.run_preflight(built.manifest.clone()).await?; @@ -73,19 +67,8 @@ pub(crate) async fn create_run( } let created_run_id = client.create_run_from_manifest(built.manifest).await?; - let local_run_dir = if target.is_unix_socket() { - Some( - Storage::new(user_config::storage_dir(ctx.machine_settings())?) - .run_scratch(&created_run_id) - .root() - .to_path_buf(), - ) - } else { - None - }; Ok(CreatedRun { run_id: created_run_id, - local_run_dir, }) } diff --git a/lib/crates/fabro-cli/src/commands/run/output.rs b/lib/crates/fabro-cli/src/commands/run/output.rs index 7e89a13d7..d44cbd3ba 100644 --- a/lib/crates/fabro-cli/src/commands/run/output.rs +++ b/lib/crates/fabro-cli/src/commands/run/output.rs @@ -133,7 +133,6 @@ pub(crate) fn api_check_report_to_local(report: &types::PreflightCheckReport) -> pub(crate) async fn print_run_summary_with_client( client: &server_client::Client, run_id: &fabro_types::RunId, - local_run_dir: Option<&Path>, styles: &Styles, printer: Printer, ) -> Result<()> { @@ -151,7 +150,7 @@ pub(crate) async fn print_run_summary_with_client( print_run_conclusion( &conclusion, run_id, - local_run_dir, + None, None, pr_url.as_deref(), styles, @@ -160,9 +159,7 @@ pub(crate) async fn print_run_summary_with_client( let final_output = resolve_final_output_with_client(client, run_id, checkpoint.as_ref()).await?; print_final_output(final_output.as_deref(), styles, printer); - if local_run_dir.is_some() { - print_assets_with_client(client, run_id, styles, printer).await?; - } + print_assets_with_client(client, run_id, styles, printer).await?; Ok(()) } diff --git a/lib/crates/fabro-cli/src/commands/run/resume.rs b/lib/crates/fabro-cli/src/commands/run/resume.rs index 1aa723459..85d801474 100644 --- a/lib/crates/fabro-cli/src/commands/run/resume.rs +++ b/lib/crates/fabro-cli/src/commands/run/resume.rs @@ -43,14 +43,8 @@ pub(crate) async fn resume_command( )) .await?; if !json { - super::output::print_run_summary_with_client( - client.as_ref(), - &run_id, - None, - styles, - printer, - ) - .await?; + super::output::print_run_summary_with_client(client.as_ref(), &run_id, styles, printer) + .await?; } if exit_code != std::process::ExitCode::SUCCESS { std::process::exit(1); diff --git a/lib/crates/fabro-cli/src/commands/server/foreground.rs b/lib/crates/fabro-cli/src/commands/server/foreground.rs index f2b536281..246eadd64 100644 --- a/lib/crates/fabro-cli/src/commands/server/foreground.rs +++ b/lib/crates/fabro-cli/src/commands/server/foreground.rs @@ -2,7 +2,7 @@ use std::path::PathBuf; use anyhow::Result; use chrono::Utc; -use fabro_config::Storage; +use fabro_config::ServerRuntimeState; use fabro_server::bind::BindRequest; use fabro_server::serve; use fabro_server::serve::ServeArgs; @@ -35,7 +35,7 @@ pub(crate) async fn execute( None }; - let log_path = Storage::new(&storage_dir).server_state().log_path(); + let log_path = ServerRuntimeState::new(&storage_dir).log_path(); let dev_token_path = std::env::var_os("FABRO_DEV_TOKEN_PATH").map(PathBuf::from); let pid = std::process::id(); diff --git a/lib/crates/fabro-cli/src/commands/server/mod.rs b/lib/crates/fabro-cli/src/commands/server/mod.rs index 87bf6695d..df730f5bf 100644 --- a/lib/crates/fabro-cli/src/commands/server/mod.rs +++ b/lib/crates/fabro-cli/src/commands/server/mod.rs @@ -22,7 +22,7 @@ use crate::args::{ GlobalArgs, ServerCommand, ServerRestartArgs, ServerServeArgs, ServerStartArgs, ServerStatusArgs, ServerStopArgs, }; -use crate::user_config; +use crate::{local_server, user_config}; pub(crate) async fn dispatch( command: ServerCommand, @@ -54,9 +54,8 @@ pub(crate) async fn dispatch( serve_args.config.as_deref(), storage_dir.as_deref(), )?; - let storage_dir = user_config::storage_dir(&settings)?; - let bind_addr = - serve::resolve_bind_request_from_settings(&settings, serve_args.bind.as_deref())?; + let storage_dir = local_server::storage_dir(&settings)?; + let bind_addr = local_server::bind_request(&settings, serve_args.bind.as_deref())?; let styles: &'static Styles = Box::leak(Box::new(Styles::detect_stderr())); Box::pin(start::execute( bind_addr, @@ -74,7 +73,7 @@ pub(crate) async fn dispatch( timeout, }) => { let settings = user_config::load_settings_with_storage_dir(storage_dir.as_deref())?; - let storage_dir = user_config::storage_dir(&settings)?; + let storage_dir = local_server::storage_dir(&settings)?; stop::execute(&storage_dir, Duration::from_secs(timeout), printer).await } ServerCommand::Restart(ServerRestartArgs { @@ -102,10 +101,9 @@ pub(crate) async fn dispatch( serve_args.config.as_deref(), storage_dir.as_deref(), )?; - let storage_dir = user_config::storage_dir(&settings)?; + let storage_dir = local_server::storage_dir(&settings)?; stop::stop_server(&storage_dir, Duration::from_secs(timeout)).await?; - let bind_addr = - serve::resolve_bind_request_from_settings(&settings, serve_args.bind.as_deref())?; + let bind_addr = local_server::bind_request(&settings, serve_args.bind.as_deref())?; let styles: &'static Styles = Box::leak(Box::new(Styles::detect_stderr())); Box::pin(start::execute( bind_addr, @@ -120,7 +118,7 @@ pub(crate) async fn dispatch( } ServerCommand::Status(ServerStatusArgs { storage_dir, json }) => { let settings = user_config::load_settings_with_storage_dir(storage_dir.as_deref())?; - let storage_dir = user_config::storage_dir(&settings)?; + let storage_dir = local_server::storage_dir(&settings)?; status::execute(&storage_dir, json, printer) } ServerCommand::Serve(ServerServeArgs { @@ -138,9 +136,8 @@ pub(crate) async fn dispatch( .clone() .unwrap_or_else(|| user_config::active_settings_path(None)), ); - let storage_dir = user_config::storage_dir(&settings)?; - let bind_addr = - serve::resolve_bind_request_from_settings(&settings, serve_args.bind.as_deref())?; + let storage_dir = local_server::storage_dir(&settings)?; + let bind_addr = local_server::bind_request(&settings, serve_args.bind.as_deref())?; let styles: &'static Styles = Box::leak(Box::new(Styles::detect_stderr())); Box::pin(foreground::execute( record_path, diff --git a/lib/crates/fabro-cli/src/commands/server/record.rs b/lib/crates/fabro-cli/src/commands/server/record.rs index 23481d8dd..079c8a1d3 100644 --- a/lib/crates/fabro-cli/src/commands/server/record.rs +++ b/lib/crates/fabro-cli/src/commands/server/record.rs @@ -7,7 +7,7 @@ use std::path::{Path, PathBuf}; use anyhow::{Context, Result, bail}; use chrono::{DateTime, Utc}; -use fabro_config::Storage; +use fabro_config::ServerRuntimeState; use fabro_config::user::default_storage_dir; use fabro_server::bind::Bind; use fabro_util::Home; @@ -52,7 +52,7 @@ pub(crate) fn server_record_is_running(record: &ServerRecord) -> bool { } fn server_record_path(storage_dir: &Path) -> PathBuf { - Storage::new(storage_dir).server_state().record_path() + ServerRuntimeState::new(storage_dir).record_path() } fn legacy_record_path(storage_dir: &Path) -> Option { @@ -140,7 +140,7 @@ mod tests { #[test] fn write_and_read_round_trip() { let dir = tempfile::tempdir().unwrap(); - let path = Storage::new(dir.path()).server_state().record_path(); + let path = ServerRuntimeState::new(dir.path()).record_path(); let record = test_record(Bind::Tcp("127.0.0.1:3000".parse().unwrap())); write_server_record(&path, &record).unwrap(); @@ -158,7 +158,7 @@ mod tests { #[test] fn active_server_record_cleans_stale_dead_pid() { let dir = tempfile::tempdir().unwrap(); - let path = Storage::new(dir.path()).server_state().record_path(); + let path = ServerRuntimeState::new(dir.path()).record_path(); let mut record = test_record(Bind::Tcp("127.0.0.1:3000".parse().unwrap())); record.pid = u32::MAX; // definitely not alive write_server_record(&path, &record).unwrap(); diff --git a/lib/crates/fabro-cli/src/commands/server/start.rs b/lib/crates/fabro-cli/src/commands/server/start.rs index 31ebc0ce5..ddc1816a1 100644 --- a/lib/crates/fabro-cli/src/commands/server/start.rs +++ b/lib/crates/fabro-cli/src/commands/server/start.rs @@ -9,7 +9,7 @@ use std::time::Duration; use anyhow::{Context, Result, anyhow, bail}; use chrono::Utc; use fabro_config::user::{FABRO_CONFIG_ENV, default_settings_path, load_settings_config}; -use fabro_config::{Storage, envfile, resolve_server_from_file}; +use fabro_config::{ServerRuntimeState, envfile}; use fabro_server::bind::{Bind, BindRequest}; use fabro_server::jwt_auth::auth_method_name; use fabro_server::serve; @@ -23,6 +23,7 @@ use tokio::task::spawn_blocking; use tokio::time; use super::record; +use crate::local_server; pub(crate) struct ForegroundServerLogBootstrap { #[expect(dead_code, reason = "held for its Drop to release the server lock")] @@ -76,7 +77,7 @@ pub(crate) async fn prepare_foreground_server_log( ); } - let log_path = Storage::new(storage_dir).server_state().log_path(); + let log_path = ServerRuntimeState::new(storage_dir).log_path(); if let Some(parent) = log_path.parent() { std::fs::create_dir_all(parent) .with_context(|| format!("creating log directory {}", parent.display()))?; @@ -148,7 +149,7 @@ async fn ensure_server_running_with_bind( bind_request } else { let settings = load_settings_config(Some(config_path))?; - serve::resolve_bind_request_from_settings(&settings, None)? + local_server::bind_request(&settings, None)? }; match execute_daemon( @@ -222,8 +223,8 @@ fn load_or_create_local_dev_token(storage_dir: &Path, home: &Home) -> Result Result Result Result { return Ok(secret); } - let storage = Storage::new(storage_dir); - let server_env_path = storage.server_state().env_path(); + let server_env_path = ServerRuntimeState::new(storage_dir).env_path(); if let Some(secret) = envfile::read_env_file(&server_env_path) .ok() .and_then(|entries| entries.get("SESSION_SECRET").cloned()) @@ -320,9 +310,9 @@ async fn execute_foreground( }, ); - let server_state = Storage::new(&storage_dir).server_state(); - let record_path = server_state.record_path(); - let log_path = server_state.log_path(); + let runtime_state = ServerRuntimeState::new(&storage_dir); + let record_path = runtime_state.record_path(); + let log_path = runtime_state.log_path(); let pid = std::process::id(); let _record_guard = scopeguard::guard(record_path.clone(), |path| { @@ -382,14 +372,14 @@ async fn execute_daemon( return Ok(()); } - let server_state = Storage::new(storage_dir).server_state(); - let log_path = server_state.log_path(); + let runtime_state = ServerRuntimeState::new(storage_dir); + let log_path = runtime_state.log_path(); if let Some(parent) = log_path.parent() { std::fs::create_dir_all(parent) .with_context(|| format!("creating log directory {}", parent.display()))?; } - let record_path = server_state.record_path(); + let record_path = runtime_state.record_path(); let log_file = std::fs::File::create(&log_path) .with_context(|| format!("creating server log file {}", log_path.display()))?; let stdout_log = log_file @@ -517,8 +507,7 @@ fn print_auth_methods(printer: Printer, serve_args: &ServeArgs) { let settings = load_settings_config(serve_args.config.as_deref()).ok(); let auth_methods = settings .as_ref() - .and_then(|s| resolve_server_from_file(s).ok()) - .map(|s| s.auth.methods) + .map(local_server::auth_methods) .unwrap_or_default(); let names: Vec<&str> = auth_methods.iter().map(|m| auth_method_name(*m)).collect(); fabro_util::printerr!(printer, "Auth: {}", names.join(", ")); @@ -534,7 +523,7 @@ fn print_dev_token(printer: Printer, home: &Home, token: &str) { // --------------------------------------------------------------------------- async fn acquire_lock(storage_dir: &Path) -> Result { - let lock_path = Storage::new(storage_dir).server_state().lock_path(); + let lock_path = ServerRuntimeState::new(storage_dir).lock_path(); if let Some(parent) = lock_path.parent() { std::fs::create_dir_all(parent) .with_context(|| format!("creating server lock directory {}", parent.display()))?; diff --git a/lib/crates/fabro-cli/src/commands/uninstall.rs b/lib/crates/fabro-cli/src/commands/uninstall.rs index d4a9fa1ec..52a4b25e6 100644 --- a/lib/crates/fabro-cli/src/commands/uninstall.rs +++ b/lib/crates/fabro-cli/src/commands/uninstall.rs @@ -62,8 +62,20 @@ pub(crate) async fn run_uninstall( let storage_dir = user_config::load_settings().map_or_else( |_| user_config::default_storage_dir(), |settings| { - user_config::storage_dir(&settings) - .unwrap_or_else(|_| user_config::default_storage_dir()) + // Uninstall is a documented server-host exception: it may read + // [server.*] directly. + fabro_config::resolve_server_from_file(&settings) + .ok() + .and_then(|resolved| { + resolved + .storage + .root + .resolve(|name| std::env::var(name).ok()) + .ok() + }) + .map_or_else(user_config::default_storage_dir, |resolved_root| { + PathBuf::from(resolved_root.value) + }) }, ); diff --git a/lib/crates/fabro-cli/src/local_server.rs b/lib/crates/fabro-cli/src/local_server.rs new file mode 100644 index 000000000..1c087c382 --- /dev/null +++ b/lib/crates/fabro-cli/src/local_server.rs @@ -0,0 +1,65 @@ +//! Helpers for CLI code that manages the local Fabro server on this host. +//! +//! This module is the only generic CLI lifecycle surface allowed to read +//! `[server.*]` settings. User-facing CLI commands outside same-host server +//! lifecycle should not call into it. + +use std::path::PathBuf; + +use anyhow::Result; +use fabro_config::ServerRuntimeState; +use fabro_server::bind::BindRequest; +use fabro_server::serve::resolve_bind_request_from_settings; +use fabro_types::settings::{ServerAuthMethod, SettingsLayer}; + +fn render_server_resolve_errors(errors: Vec) -> anyhow::Error { + anyhow::anyhow!( + "failed to resolve server settings:\n{}", + errors + .into_iter() + .map(|error| error.to_string()) + .collect::>() + .join("\n") + ) +} + +pub(crate) fn storage_dir(settings: &SettingsLayer) -> Result { + let resolved = + fabro_config::resolve_server_from_file(settings).map_err(render_server_resolve_errors)?; + let resolved_root = resolved + .storage + .root + .resolve(|name| std::env::var(name).ok()) + .map_err(|err| { + anyhow::anyhow!( + "failed to resolve {}: {err}", + resolved.storage.root.as_source() + ) + })?; + Ok(PathBuf::from(resolved_root.value)) +} + +pub(crate) fn runtime_state(settings: &SettingsLayer) -> Result { + Ok(ServerRuntimeState::new(storage_dir(settings)?)) +} + +pub(crate) fn bind_request( + settings: &SettingsLayer, + cli_override: Option<&str>, +) -> Result { + resolve_bind_request_from_settings(settings, cli_override) +} + +pub(crate) fn auth_methods(settings: &SettingsLayer) -> Vec { + fabro_config::resolve_server_from_file(settings) + .map(|resolved| resolved.auth.methods) + .unwrap_or_default() +} + +pub(crate) fn config_log_level(settings: &SettingsLayer) -> Option { + settings + .server + .as_ref() + .and_then(|server| server.logging.as_ref()) + .and_then(|logging| logging.level.clone()) +} diff --git a/lib/crates/fabro-cli/src/main.rs b/lib/crates/fabro-cli/src/main.rs index fc64910b4..017e16f6e 100644 --- a/lib/crates/fabro-cli/src/main.rs +++ b/lib/crates/fabro-cli/src/main.rs @@ -8,6 +8,7 @@ mod command_context; mod commands; mod gh; mod landing; +mod local_server; mod logging; mod manifest_builder; mod server_client; @@ -479,8 +480,8 @@ async fn prepare_server_bootstrap( ) -> Result { let settings = user_config::load_settings_with_config_and_storage_dir(config_path, storage_dir)?; - let storage_dir = user_config::storage_dir(&settings)?; - let storage = fabro_config::Storage::new(&storage_dir); + let storage_dir = local_server::storage_dir(&settings)?; + let runtime_state = local_server::runtime_state(&settings)?; let foreground_server_log_bootstrap = if foreground { Some(commands::server::start::prepare_foreground_server_log(&storage_dir).await?) } else { @@ -489,21 +490,13 @@ async fn prepare_server_bootstrap( Ok(PreTracingBootstrap { sink: logging::InternalLogSink::Server { - path: storage.server_state().log_path(), + path: runtime_state.log_path(), }, - config_log_level: server_config_log_level(&settings), + config_log_level: local_server::config_log_level(&settings), foreground_server_log_bootstrap, }) } -fn server_config_log_level(settings: &SettingsLayer) -> Option { - settings - .server - .as_ref() - .and_then(|server| server.logging.as_ref()) - .and_then(|logging| logging.level.clone()) -} - #[cfg(test)] #[expect( clippy::disallowed_methods, diff --git a/lib/crates/fabro-cli/src/server_client.rs b/lib/crates/fabro-cli/src/server_client.rs index 337a34530..9fbccdf8e 100644 --- a/lib/crates/fabro-cli/src/server_client.rs +++ b/lib/crates/fabro-cli/src/server_client.rs @@ -8,7 +8,7 @@ use fabro_client::{ apply_bearer_token_auth, }; pub(crate) use fabro_client::{Client, RunEventStream}; -use fabro_config::Storage; +use fabro_config::ServerRuntimeState; use fabro_server::bind::Bind; pub(crate) use fabro_types::RunProjection; use fabro_types::settings::SettingsLayer; @@ -18,6 +18,7 @@ use tokio::time::sleep; use crate::args::ServerTargetArgs; use crate::commands::server::{record, start}; +use crate::local_server; use crate::user_config::{self, cli_http_client_builder}; #[derive(Debug)] @@ -69,7 +70,7 @@ pub(crate) async fn connect_server_with_settings( if let Some(path) = target.as_unix_socket_path() { return connect_managed_unix_socket_api_client_bundle( path, - &user_config::storage_dir(settings)?, + &local_server::storage_dir(settings)?, base_config_path, ) .await; @@ -77,7 +78,7 @@ pub(crate) async fn connect_server_with_settings( return connect_target_api_client_bundle(&target).await; } - connect_local_api_client_bundle(&user_config::storage_dir(settings)?, base_config_path).await + connect_local_api_client_bundle(&local_server::storage_dir(settings)?, base_config_path).await } async fn connect_managed_unix_socket_api_client_bundle( @@ -230,12 +231,13 @@ fn load_dev_token_if_available_from_sources( } if let Some(storage_dir) = storage_dir { - let storage_token_path = Storage::new(storage_dir).server_state().dev_token_path(); + let runtime_state = ServerRuntimeState::new(storage_dir); + let storage_token_path = runtime_state.dev_token_path(); if let Some(token) = dev_token::read_dev_token_file(&storage_token_path) { return Some(token); } - let record_path = Storage::new(storage_dir).server_state().record_path(); + let record_path = runtime_state.record_path(); if let Some(token) = record::read_server_record(&record_path) .and_then(|server| server.dev_token_path) .as_deref() @@ -413,15 +415,12 @@ mod tests { let token_path = token_dir.path().join("dev-token"); std::fs::write(&token_path, token).unwrap(); - let record_path = fabro_config::Storage::new(storage.path()) - .server_state() - .record_path(); + let runtime_state = fabro_config::ServerRuntimeState::new(storage.path()); + let record_path = runtime_state.record_path(); record::write_server_record(&record_path, &record::ServerRecord { pid: std::process::id(), bind: Bind::Unix(temp_home.path().join("fabro.sock")), - log_path: fabro_config::Storage::new(storage.path()) - .server_state() - .log_path(), + log_path: runtime_state.log_path(), dev_token_path: Some(token_path), started_at: Utc::now(), }) diff --git a/lib/crates/fabro-cli/src/user_config.rs b/lib/crates/fabro-cli/src/user_config.rs index 3fa3710da..5a62e6cc6 100644 --- a/lib/crates/fabro-cli/src/user_config.rs +++ b/lib/crates/fabro-cli/src/user_config.rs @@ -10,6 +10,7 @@ use fabro_util::version::FABRO_VERSION; use tracing::debug; use crate::args::ServerTargetArgs; +use crate::local_server; pub(crate) fn load_settings() -> anyhow::Result { load_settings_with_config_and_storage_dir(None, None) @@ -83,28 +84,11 @@ pub(crate) fn default_server_target() -> ServerTarget { ServerTarget::unix_socket_path(default_socket_path()).expect("default socket path is absolute") } +#[deprecated( + note = "use local_server::storage_dir for lifecycle; PR commands must move to server-side API" +)] pub(crate) fn storage_dir(settings: &SettingsLayer) -> anyhow::Result { - let resolved = fabro_config::resolve_server_from_file(settings).map_err(|errors| { - anyhow::anyhow!( - "failed to resolve server settings:\n{}", - errors - .into_iter() - .map(|error| error.to_string()) - .collect::>() - .join("\n") - ) - })?; - let resolved_root = resolved - .storage - .root - .resolve(|name| std::env::var(name).ok()) - .map_err(|err| { - anyhow::anyhow!( - "failed to resolve {}: {err}", - resolved.storage.root.as_source() - ) - })?; - Ok(PathBuf::from(resolved_root.value)) + local_server::storage_dir(settings) } fn parse_server_target(value: &str) -> Result { diff --git a/lib/crates/fabro-cli/tests/it/cmd/install.rs b/lib/crates/fabro-cli/tests/it/cmd/install.rs index 8699fdf82..595e9542e 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/install.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/install.rs @@ -237,7 +237,7 @@ mode = "keep-me" ), ); - let server_env_path = Storage::new(&storage_dir).server_state().env_path(); + let server_env_path = Storage::new(&storage_dir).runtime_state().env_path(); envfile::write_env_file( &server_env_path, &std::collections::HashMap::from([ diff --git a/lib/crates/fabro-cli/tests/it/cmd/run.rs b/lib/crates/fabro-cli/tests/it/cmd/run.rs index ddcb9e400..f72961e68 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/run.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/run.rs @@ -530,6 +530,13 @@ fn remote_foreground_run_consumes_paginated_events_and_prints_server_backed_summ .header("Content-Type", "application/json") .body(remote_run_state_response().to_string()); }); + server.mock(|when, then| { + when.method("GET") + .path(format!("/api/v1/runs/{run_id}/artifacts")); + then.status(200) + .header("Content-Type", "application/json") + .body(serde_json::json!({ "data": [] }).to_string()); + }); let workflow = context.install_fixture("simple.fabro"); let output = context @@ -665,7 +672,6 @@ fn dry_run_simple() { Run: [ULID] Status: SUCCESS Duration: [DURATION] - Run: [RUN_DIR] === Output === [Simulated] Response for stage: report diff --git a/lib/crates/fabro-cli/tests/it/cmd/support.rs b/lib/crates/fabro-cli/tests/it/cmd/support.rs index 04b470048..2f5602286 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/support.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/support.rs @@ -666,7 +666,7 @@ struct TestServerRecord { } pub(crate) fn local_dev_token(storage_dir: &Path) -> Option { - let server_state = Storage::new(storage_dir).server_state(); + let server_state = Storage::new(storage_dir).runtime_state(); fabro_util::dev_token::read_dev_token_file(&server_state.dev_token_path()).or_else(|| { std::fs::read_to_string(server_state.record_path()) @@ -679,7 +679,7 @@ pub(crate) fn local_dev_token(storage_dir: &Path) -> Option { } pub(crate) fn server_endpoint(storage_dir: &Path) -> Option<(fabro_http::HttpClient, String)> { - let record_path = Storage::new(storage_dir).server_state().record_path(); + let record_path = Storage::new(storage_dir).runtime_state().record_path(); let record = std::fs::read_to_string(record_path) .ok() .and_then(|content| serde_json::from_str::(&content).ok())?; @@ -714,7 +714,7 @@ pub(crate) fn server_endpoint(storage_dir: &Path) -> Option<(fabro_http::HttpCli } pub(crate) fn server_target(storage_dir: &Path) -> String { - let record_path = Storage::new(storage_dir).server_state().record_path(); + let record_path = Storage::new(storage_dir).runtime_state().record_path(); let record = std::fs::read_to_string(record_path) .ok() .and_then(|content| serde_json::from_str::(&content).ok()) diff --git a/lib/crates/fabro-cli/tests/it/workflow/dry_run_examples.rs b/lib/crates/fabro-cli/tests/it/workflow/dry_run_examples.rs index ec0508cb9..a44ef957d 100644 --- a/lib/crates/fabro-cli/tests/it/workflow/dry_run_examples.rs +++ b/lib/crates/fabro-cli/tests/it/workflow/dry_run_examples.rs @@ -32,7 +32,6 @@ fn dry_run_branching() { Run: [ULID] Status: SUCCESS Duration: [DURATION] - Run: [RUN_DIR] === Output === [Simulated] Response for stage: validate @@ -66,7 +65,6 @@ fn dry_run_conditions() { Run: [ULID] Status: SUCCESS Duration: [DURATION] - Run: [RUN_DIR] === Output === [Simulated] Response for stage: path_b @@ -101,7 +99,6 @@ fn dry_run_parallel() { Run: [ULID] Status: SUCCESS Duration: [DURATION] - Run: [RUN_DIR] === Output === [Simulated] Response for stage: review @@ -136,7 +133,6 @@ fn dry_run_styled() { Run: [ULID] Status: SUCCESS Duration: [DURATION] - Run: [RUN_DIR] === Output === [Simulated] Response for stage: critical_review @@ -169,6 +165,5 @@ fn dry_run_legacy_tool() { Run: [ULID] Status: SUCCESS Duration: [DURATION] - Run: [RUN_DIR] "); } diff --git a/lib/crates/fabro-config/src/lib.rs b/lib/crates/fabro-config/src/lib.rs index 08f25ac21..7f53d50a5 100644 --- a/lib/crates/fabro-config/src/lib.rs +++ b/lib/crates/fabro-config/src/lib.rs @@ -38,7 +38,7 @@ pub use resolve::{ resolve_workflow_from_file, }; use serde::de::DeserializeOwned; -pub use storage::{RunScratch, ServerState, Storage}; +pub use storage::{RunScratch, ServerRuntimeState, Storage}; pub fn load_and_resolve( layers: effective_settings::EffectiveSettingsLayers, diff --git a/lib/crates/fabro-config/src/storage.rs b/lib/crates/fabro-config/src/storage.rs index 2522096f8..84bf576cb 100644 --- a/lib/crates/fabro-config/src/storage.rs +++ b/lib/crates/fabro-config/src/storage.rs @@ -9,7 +9,7 @@ pub struct Storage { } #[derive(Clone, Debug, PartialEq, Eq)] -pub struct ServerState { +pub struct ServerRuntimeState { root: PathBuf, } @@ -29,11 +29,6 @@ impl Storage { &self.root } - #[must_use] - pub fn logs_dir(&self) -> PathBuf { - self.root.join("logs") - } - #[must_use] pub fn cache_dir(&self) -> PathBuf { self.root.join("cache") @@ -53,8 +48,8 @@ impl Storage { } #[must_use] - pub fn server_state(&self) -> ServerState { - ServerState::new(self.root.clone()) + pub fn runtime_state(&self) -> ServerRuntimeState { + ServerRuntimeState::new(self.root.clone()) } #[must_use] @@ -83,12 +78,17 @@ impl Storage { } } -impl ServerState { +impl ServerRuntimeState { #[must_use] pub fn new(root: impl Into) -> Self { Self { root: root.into() } } + #[must_use] + pub fn logs_dir(&self) -> PathBuf { + self.root.join("logs") + } + #[must_use] pub fn record_path(&self) -> PathBuf { self.root.join("server.json") @@ -101,7 +101,7 @@ impl ServerState { #[must_use] pub fn log_path(&self) -> PathBuf { - self.root.join("logs").join("server.log") + self.logs_dir().join("server.log") } #[must_use] @@ -163,17 +163,14 @@ mod tests { use chrono::Local; use fabro_types::RunId; - use super::{RunScratch, Storage}; + use super::{RunScratch, ServerRuntimeState, Storage}; #[test] fn storage_accessors_are_relative_to_root() { let storage = Storage::new("/tmp/fabro-data"); + let runtime = ServerRuntimeState::new("/tmp/fabro-data"); assert_eq!(storage.root(), std::path::Path::new("/tmp/fabro-data")); - assert_eq!( - storage.logs_dir(), - std::path::Path::new("/tmp/fabro-data/logs") - ); assert_eq!( storage.cache_dir(), std::path::Path::new("/tmp/fabro-data/cache") @@ -199,19 +196,23 @@ mod tests { std::path::Path::new("/tmp/fabro-data/objects/artifacts") ); assert_eq!( - storage.server_state().record_path(), + runtime.logs_dir(), + std::path::Path::new("/tmp/fabro-data/logs") + ); + assert_eq!( + runtime.record_path(), std::path::Path::new("/tmp/fabro-data/server.json") ); assert_eq!( - storage.server_state().lock_path(), + runtime.lock_path(), std::path::Path::new("/tmp/fabro-data/server.lock") ); assert_eq!( - storage.server_state().log_path(), + runtime.log_path(), std::path::Path::new("/tmp/fabro-data/logs/server.log") ); assert_eq!( - storage.server_state().env_path(), + runtime.env_path(), std::path::Path::new("/tmp/fabro-data/server.env") ); } diff --git a/lib/crates/fabro-install/src/lib.rs b/lib/crates/fabro-install/src/lib.rs index 2e9302f6f..31a68b838 100644 --- a/lib/crates/fabro-install/src/lib.rs +++ b/lib/crates/fabro-install/src/lib.rs @@ -266,7 +266,7 @@ fn persist_server_env_secrets(storage_dir: &Path, secrets: &[(String, String)]) return Ok(()); } - let env_path = Storage::new(storage_dir).server_state().env_path(); + let env_path = Storage::new(storage_dir).runtime_state().env_path(); envfile::merge_env_file(&env_path, secrets.iter().cloned()) .with_context(|| format!("merging server env secrets into {}", env_path.display()))?; Ok(()) @@ -500,7 +500,7 @@ name = "custom" assert_eq!(restored.get("EXISTING_SECRET"), Some("keep")); assert_eq!(restored.get("bad-secret-name"), None); - let server_env = envfile::read_env_file(&storage.server_state().env_path()).unwrap(); + let server_env = envfile::read_env_file(&storage.runtime_state().env_path()).unwrap(); assert_eq!( server_env.get("SESSION_SECRET").map(String::as_str), Some("session") diff --git a/lib/crates/fabro-server/src/diagnostics.rs b/lib/crates/fabro-server/src/diagnostics.rs index 6123adc5c..582816a76 100644 --- a/lib/crates/fabro-server/src/diagnostics.rs +++ b/lib/crates/fabro-server/src/diagnostics.rs @@ -68,7 +68,7 @@ pub async fn run_all(state: &AppState) -> DiagnosticsReport { }, CheckSection { title: "Configuration".to_string(), - checks: vec![crypto], + checks: vec![crypto, check_storage_dir(state)], }, ], } @@ -398,6 +398,45 @@ fn check_sandbox(state: &AppState) -> CheckResult { } } +fn check_storage_dir(state: &AppState) -> CheckResult { + check_storage_dir_path(&state.server_storage_dir()) +} + +#[expect( + clippy::disallowed_methods, + reason = "Server diagnostics deliberately performs a synchronous local filesystem probe." +)] +fn check_storage_dir_path(path: &std::path::Path) -> CheckResult { + let exists = path.is_dir(); + let readable = exists && std::fs::read_dir(path).is_ok(); + let writable = exists && tempfile::tempfile_in(path).is_ok(); + let details = vec![ + CheckDetail::new(format!("Exists: {}", if exists { "yes" } else { "no" })), + CheckDetail::new(format!("Readable: {}", if readable { "yes" } else { "no" })), + CheckDetail::new(format!("Writable: {}", if writable { "yes" } else { "no" })), + ]; + let is_healthy = exists && readable && writable; + let display = path.display(); + + CheckResult { + name: "Storage directory".to_string(), + status: if is_healthy { + CheckStatus::Pass + } else { + CheckStatus::Error + }, + summary: display.to_string(), + details, + remediation: if is_healthy { + None + } else if !exists { + Some(format!("Create the directory: mkdir -p {display}")) + } else { + Some(format!("Fix permissions on {display}")) + }, + } +} + async fn check_brave_search(state: &AppState) -> CheckResult { let Some(api_key) = state.vault_or_env("BRAVE_SEARCH_API_KEY") else { return CheckResult { @@ -543,3 +582,39 @@ fn check_crypto(state: &AppState) -> CheckResult { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn check_storage_dir_path_passes_for_readable_writable_directory() { + let dir = tempfile::tempdir().unwrap(); + + let result = check_storage_dir_path(dir.path()); + + assert_eq!(result.name, "Storage directory"); + assert_eq!(result.status, CheckStatus::Pass); + assert_eq!(result.summary, dir.path().display().to_string()); + assert!(result.remediation.is_none()); + } + + #[test] + fn check_storage_dir_path_errors_for_missing_directory() { + let dir = tempfile::tempdir().unwrap(); + let missing = dir.path().join("missing"); + + let result = check_storage_dir_path(&missing); + + assert_eq!(result.name, "Storage directory"); + assert_eq!(result.status, CheckStatus::Error); + assert_eq!(result.summary, missing.display().to_string()); + assert_eq!( + result.remediation, + Some(format!( + "Create the directory: mkdir -p {}", + missing.display() + )) + ); + } +} diff --git a/lib/crates/fabro-server/src/install.rs b/lib/crates/fabro-server/src/install.rs index 9448181df..b5fe903df 100644 --- a/lib/crates/fabro-server/src/install.rs +++ b/lib/crates/fabro-server/src/install.rs @@ -826,7 +826,7 @@ async fn post_install_finish( }; if let Err(err) = dev_token::write_dev_token( &Storage::new(state.storage_dir.as_ref()) - .server_state() + .runtime_state() .dev_token_path(), &token, ) { diff --git a/lib/crates/fabro-server/src/serve.rs b/lib/crates/fabro-server/src/serve.rs index f6c27856b..6e6917d3d 100644 --- a/lib/crates/fabro-server/src/serve.rs +++ b/lib/crates/fabro-server/src/serve.rs @@ -469,7 +469,7 @@ where }; let storage = Storage::new(&data_dir); let vault_path = storage.secrets_path(); - let server_env_path = storage.server_state().env_path(); + let server_env_path = storage.runtime_state().env_path(); let server_secrets = ServerSecrets::load(server_env_path.clone())?; let webhook_secret_present = server_secrets.get(WEBHOOK_SECRET_ENV).is_some(); diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index 19ae6a41f..11b57c6e5 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -1555,7 +1555,7 @@ fn build_disk_usage_response( verbose: bool, ) -> anyhow::Result { let scratch_base_dir = scratch_base(storage_dir); - let logs_base_dir = Storage::new(storage_dir).logs_dir(); + let logs_base_dir = Storage::new(storage_dir).runtime_state().logs_dir(); let runs = scan_runs_with_summaries(summaries, &scratch_base_dir)?; let mut active_count = 0u64; @@ -3732,7 +3732,7 @@ struct WorkerServerRecord { } fn current_server_target(storage_dir: &std::path::Path) -> anyhow::Result { - let record_path = Storage::new(storage_dir).server_state().record_path(); + let record_path = Storage::new(storage_dir).runtime_state().record_path(); #[expect( clippy::disallowed_methods, reason = "sync helper invoked from worker_command (sync) via spawn_blocking at the async \ diff --git a/lib/crates/fabro-server/tests/it/api/install.rs b/lib/crates/fabro-server/tests/it/api/install.rs index 07b074b7e..52082d254 100644 --- a/lib/crates/fabro-server/tests/it/api/install.rs +++ b/lib/crates/fabro-server/tests/it/api/install.rs @@ -395,7 +395,7 @@ async fn token_install_finish_persists_settings_env_and_vault() { let server_env = std::fs::read_to_string( fabro_config::Storage::new(temp_dir.path()) - .server_state() + .runtime_state() .env_path(), ) .unwrap(); @@ -556,7 +556,7 @@ async fn app_install_finish_omits_dev_token_and_does_not_write_it() { ); let server_env = - std::fs::read_to_string(Storage::new(temp_dir.path()).server_state().env_path()).unwrap(); + std::fs::read_to_string(Storage::new(temp_dir.path()).runtime_state().env_path()).unwrap(); assert!(!server_env.contains("FABRO_DEV_TOKEN=")); assert!( @@ -565,7 +565,7 @@ async fn app_install_finish_omits_dev_token_and_does_not_write_it() { ); assert!( !Storage::new(temp_dir.path()) - .server_state() + .runtime_state() .dev_token_path() .exists(), "storage dev token file should not be created for App installs" @@ -1372,7 +1372,7 @@ async fn install_finish_failure_restores_settings_and_vault_but_leaves_env_keys( "{ not valid json" ); - let server_env = std::fs::read_to_string(storage.server_state().env_path()).unwrap(); + let server_env = std::fs::read_to_string(storage.runtime_state().env_path()).unwrap(); assert!(server_env.contains("SESSION_SECRET=")); assert!(server_env.contains("FABRO_DEV_TOKEN=")); assert!(!callback_invoked.load(Ordering::Acquire)); @@ -1441,10 +1441,10 @@ async fn install_finish_failure_leaves_home_dev_token_mirror_written() { let home_dev_token = dev_token::read_dev_token_file(&home.dev_token_path()) .expect("home dev token should exist"); let storage_dev_token = - dev_token::read_dev_token_file(&storage.server_state().dev_token_path()) + dev_token::read_dev_token_file(&storage.runtime_state().dev_token_path()) .expect("storage dev token should exist"); assert_eq!(home_dev_token, storage_dev_token); - let server_env = std::fs::read_to_string(storage.server_state().env_path()).unwrap(); + let server_env = std::fs::read_to_string(storage.runtime_state().env_path()).unwrap(); assert!(server_env.contains(&format!("FABRO_DEV_TOKEN={home_dev_token}"))); } diff --git a/lib/crates/fabro-server/tests/it/api/tcp.rs b/lib/crates/fabro-server/tests/it/api/tcp.rs index 4fbdbdbb0..9ff9a8f36 100644 --- a/lib/crates/fabro-server/tests/it/api/tcp.rs +++ b/lib/crates/fabro-server/tests/it/api/tcp.rs @@ -9,7 +9,7 @@ use std::sync::Arc; use std::time::Duration; use axum::http::StatusCode; -use fabro_config::{ServerState, parse_settings_layer, resolve_server_from_file}; +use fabro_config::{ServerRuntimeState, parse_settings_layer, resolve_server_from_file}; use fabro_server::bind::Bind; use fabro_server::ip_allowlist::{IpAllowlist, IpAllowlistConfig}; use fabro_server::jwt_auth::{AuthMode, resolve_auth_mode_with_lookup}; @@ -64,7 +64,7 @@ fn write_test_config(tempdir: &TempDir, settings: &str) -> PathBuf { let config_path = tempdir.path().join("settings.toml"); std::fs::write(&config_path, settings).expect("test settings should write"); std::fs::write( - ServerState::new(tempdir.path()).env_path(), + ServerRuntimeState::new(tempdir.path()).env_path(), format!("FABRO_DEV_TOKEN={TEST_DEV_TOKEN}\nSESSION_SECRET={TEST_SESSION_SECRET}\n"), ) .expect("test env file should write"); From 6e07f688ab3d63c2da2c21e3fa22d04fe592cce7 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Wed, 22 Apr 2026 00:16:49 -0400 Subject: [PATCH 2/2] refactor(cli): collapse duplicate server-settings resolvers through local_server Route install/uninstall through local_server::storage_dir instead of hand- rolled copies, drop dead connect_api_client and run_dir plumbing, eliminate double-resolve in prepare_server_bootstrap, and tighten the boundary allowlist now that uninstall no longer needs the exemption. Co-Authored-By: Claude Opus 4.7 (1M context) --- bin/dev/check-boundary.sh | 1 - lib/crates/fabro-cli/src/commands/install.rs | 28 +++---------------- .../fabro-cli/src/commands/pr/create.rs | 2 -- lib/crates/fabro-cli/src/commands/pr/mod.rs | 4 --- .../fabro-cli/src/commands/run/output.rs | 16 +---------- .../fabro-cli/src/commands/uninstall.rs | 25 ++++------------- lib/crates/fabro-cli/src/local_server.rs | 5 ---- lib/crates/fabro-cli/src/main.rs | 2 +- lib/crates/fabro-cli/src/server_client.rs | 10 ------- 9 files changed, 11 insertions(+), 82 deletions(-) diff --git a/bin/dev/check-boundary.sh b/bin/dev/check-boundary.sh index 6842a5aea..d1cfb3f50 100755 --- a/bin/dev/check-boundary.sh +++ b/bin/dev/check-boundary.sh @@ -6,7 +6,6 @@ cd "$(dirname "$0")/../.." symbol_allowlist=( "lib/crates/fabro-cli/src/local_server.rs" "lib/crates/fabro-cli/src/commands/install.rs" - "lib/crates/fabro-cli/src/commands/uninstall.rs" "lib/crates/fabro-cli/src/commands/run/runner.rs" "lib/crates/fabro-cli/src/commands/pr/mod.rs" "lib/crates/fabro-cli/src/commands/pr/create.rs" diff --git a/lib/crates/fabro-cli/src/commands/install.rs b/lib/crates/fabro-cli/src/commands/install.rs index 18d9e43fd..776756bdd 100644 --- a/lib/crates/fabro-cli/src/commands/install.rs +++ b/lib/crates/fabro-cli/src/commands/install.rs @@ -5,7 +5,7 @@ use std::future::Future; use std::net::SocketAddr; -use std::path::{Path, PathBuf}; +use std::path::Path; use std::process::Stdio; use std::time::Duration; @@ -57,7 +57,7 @@ use crate::shared::provider_auth::{ ApiKeySource, authenticate_provider, authenticate_provider_with_api_key_source, authenticate_provider_with_method, prompt_confirm, prompt_password, provider_display_name, }; -use crate::{server_client, user_config}; +use crate::{local_server, server_client, user_config}; const GITHUB_TOKEN_SECRET_KEY: &str = "GITHUB_TOKEN"; const GITHUB_APP_PRIVATE_KEY_KEY: &str = "GITHUB_APP_PRIVATE_KEY"; @@ -1292,22 +1292,6 @@ fn render_server_resolve_errors(errors: Vec) -> anyhow::Error { ) } -fn resolved_server_storage_dir(settings: &SettingsLayer) -> Result { - let resolved = - fabro_config::resolve_server_from_file(settings).map_err(render_server_resolve_errors)?; - let resolved_root = resolved - .storage - .root - .resolve(|name| std::env::var(name).ok()) - .map_err(|err| { - anyhow::anyhow!( - "failed to resolve {}: {err}", - resolved.storage.root.as_source() - ) - })?; - Ok(PathBuf::from(resolved_root.value)) -} - async fn write_artifact_store_metadata( settings: &SettingsLayer, fabro_version: &str, @@ -1510,9 +1494,7 @@ async fn run_install_github_inner( .context("failed to parse existing settings.toml")?, args.storage_dir.as_deref(), ); - // Install is a documented server-host exception: it may read [server.*] - // directly. - let storage_dir = resolved_server_storage_dir(&parsed_settings).unwrap_or_else(|_| { + let storage_dir = local_server::storage_dir(&parsed_settings).unwrap_or_else(|_| { args.storage_dir .clone_path() .unwrap_or_else(default_storage_dir) @@ -1659,9 +1641,7 @@ async fn run_install_inner( let s = Styles::detect_stderr(); let emoji = console::Emoji("⚒️ ", ""); let cli_settings = user_config::load_settings_with_storage_dir(args.storage_dir.as_deref())?; - // Install is a documented server-host exception: it may read [server.*] - // directly. - let storage_dir = resolved_server_storage_dir(&cli_settings)?; + let storage_dir = local_server::storage_dir(&cli_settings)?; let server_was_running = record::active_server_record(&storage_dir)?.is_some(); let fabro_dir = fabro_util::Home::from_env().root().to_path_buf(); let config_path = fabro_dir.join(SETTINGS_CONFIG_FILENAME); diff --git a/lib/crates/fabro-cli/src/commands/pr/create.rs b/lib/crates/fabro-cli/src/commands/pr/create.rs index 78d9703ad..3e269ba79 100644 --- a/lib/crates/fabro-cli/src/commands/pr/create.rs +++ b/lib/crates/fabro-cli/src/commands/pr/create.rs @@ -105,8 +105,6 @@ pub(super) async fn create_command( ); } - // boundary-exempt(pr-api): remove with follow-up #1 when PR ops move - // server-side. let vault = user_config::storage_dir(ctx.machine_settings()) .ok() .and_then(|dir| Vault::load(Storage::new(&dir).secrets_path()).ok()) diff --git a/lib/crates/fabro-cli/src/commands/pr/mod.rs b/lib/crates/fabro-cli/src/commands/pr/mod.rs index 84b6a0b57..bf7a3671b 100644 --- a/lib/crates/fabro-cli/src/commands/pr/mod.rs +++ b/lib/crates/fabro-cli/src/commands/pr/mod.rs @@ -47,8 +47,6 @@ fn load_github_credentials_required( printer: Printer, ) -> Result { let ctx = CommandContext::base(printer, cli.clone(), cli_layer)?; - // boundary-exempt(pr-api): remove with follow-up #1 when PR ops move - // server-side. let server_settings = fabro_config::resolve_server_from_file(ctx.machine_settings()).map_err(|errors| { anyhow!( @@ -60,8 +58,6 @@ fn load_github_credentials_required( .join("\n") ) })?; - // boundary-exempt(pr-api): remove with follow-up #1 when PR ops move - // server-side. let vault = user_config::storage_dir(ctx.machine_settings()) .ok() .and_then(|dir| fabro_vault::Vault::load(Storage::new(&dir).secrets_path()).ok()); diff --git a/lib/crates/fabro-cli/src/commands/run/output.rs b/lib/crates/fabro-cli/src/commands/run/output.rs index d44cbd3ba..6880fcbcb 100644 --- a/lib/crates/fabro-cli/src/commands/run/output.rs +++ b/lib/crates/fabro-cli/src/commands/run/output.rs @@ -17,9 +17,7 @@ use fabro_workflow::records::Conclusion; use indicatif::HumanDuration; use crate::server_client; -use crate::shared::{ - format_tokens_human, format_usd_micros, print_diagnostics, relative_path, tilde_path, -}; +use crate::shared::{format_tokens_human, format_usd_micros, print_diagnostics, relative_path}; pub(crate) fn print_preflight_workflow_summary( workflow: &types::PreflightWorkflowSummary, @@ -151,7 +149,6 @@ pub(crate) async fn print_run_summary_with_client( &conclusion, run_id, None, - None, pr_url.as_deref(), styles, printer, @@ -166,7 +163,6 @@ pub(crate) async fn print_run_summary_with_client( pub(crate) fn print_run_conclusion( conclusion: &Conclusion, run_id: impl std::fmt::Display, - run_dir: Option<&Path>, pushed_branch: Option<&str>, pr_url: Option<&str>, styles: &Styles, @@ -248,16 +244,6 @@ pub(crate) fn print_run_conclusion( } } - if let Some(run_dir) = run_dir { - fabro_util::printerr!( - printer, - "{}", - styles - .dim - .apply_to(format!("Run: {}", tilde_path(run_dir))) - ); - } - if let Some(ref failure) = conclusion.failure_reason { fabro_util::printerr!(printer, "Failure: {}", styles.red.apply_to(failure)); } diff --git a/lib/crates/fabro-cli/src/commands/uninstall.rs b/lib/crates/fabro-cli/src/commands/uninstall.rs index 52a4b25e6..3a5be57d5 100644 --- a/lib/crates/fabro-cli/src/commands/uninstall.rs +++ b/lib/crates/fabro-cli/src/commands/uninstall.rs @@ -23,7 +23,7 @@ use tracing::warn; use crate::args::UninstallArgs; use crate::commands::server; use crate::shared::{format_size, print_json_pretty, tilde_path}; -use crate::user_config; +use crate::{local_server, user_config}; #[derive(Debug, Serialize)] struct Inventory { @@ -59,25 +59,10 @@ pub(crate) async fn run_uninstall( return Ok(()); } - let storage_dir = user_config::load_settings().map_or_else( - |_| user_config::default_storage_dir(), - |settings| { - // Uninstall is a documented server-host exception: it may read - // [server.*] directly. - fabro_config::resolve_server_from_file(&settings) - .ok() - .and_then(|resolved| { - resolved - .storage - .root - .resolve(|name| std::env::var(name).ok()) - .ok() - }) - .map_or_else(user_config::default_storage_dir, |resolved_root| { - PathBuf::from(resolved_root.value) - }) - }, - ); + let storage_dir = user_config::load_settings() + .ok() + .and_then(|settings| local_server::storage_dir(&settings).ok()) + .unwrap_or_else(user_config::default_storage_dir); let inventory = build_inventory(&home_root, &storage_dir)?; diff --git a/lib/crates/fabro-cli/src/local_server.rs b/lib/crates/fabro-cli/src/local_server.rs index 1c087c382..bef5b9617 100644 --- a/lib/crates/fabro-cli/src/local_server.rs +++ b/lib/crates/fabro-cli/src/local_server.rs @@ -7,7 +7,6 @@ use std::path::PathBuf; use anyhow::Result; -use fabro_config::ServerRuntimeState; use fabro_server::bind::BindRequest; use fabro_server::serve::resolve_bind_request_from_settings; use fabro_types::settings::{ServerAuthMethod, SettingsLayer}; @@ -39,10 +38,6 @@ pub(crate) fn storage_dir(settings: &SettingsLayer) -> Result { Ok(PathBuf::from(resolved_root.value)) } -pub(crate) fn runtime_state(settings: &SettingsLayer) -> Result { - Ok(ServerRuntimeState::new(storage_dir(settings)?)) -} - pub(crate) fn bind_request( settings: &SettingsLayer, cli_override: Option<&str>, diff --git a/lib/crates/fabro-cli/src/main.rs b/lib/crates/fabro-cli/src/main.rs index 017e16f6e..df2e44f49 100644 --- a/lib/crates/fabro-cli/src/main.rs +++ b/lib/crates/fabro-cli/src/main.rs @@ -481,7 +481,7 @@ async fn prepare_server_bootstrap( let settings = user_config::load_settings_with_config_and_storage_dir(config_path, storage_dir)?; let storage_dir = local_server::storage_dir(&settings)?; - let runtime_state = local_server::runtime_state(&settings)?; + let runtime_state = fabro_config::ServerRuntimeState::new(storage_dir.clone()); let foreground_server_log_bootstrap = if foreground { Some(commands::server::start::prepare_foreground_server_log(&storage_dir).await?) } else { diff --git a/lib/crates/fabro-cli/src/server_client.rs b/lib/crates/fabro-cli/src/server_client.rs index 9fbccdf8e..182dcad23 100644 --- a/lib/crates/fabro-cli/src/server_client.rs +++ b/lib/crates/fabro-cli/src/server_client.rs @@ -140,16 +140,6 @@ async fn connect_local_api_client_bundle( } } -#[allow( - dead_code, - reason = "Retained for pending storage-backed internal callers and referenced in existing design docs." -)] -pub(crate) async fn connect_api_client(storage_dir: &Path) -> Result { - connect_local_api_client_bundle(storage_dir, &user_config::active_settings_path(None)) - .await - .map(|client| client.api_client()) -} - async fn connect_target_api_client_bundle(target: &ServerTarget) -> Result { let credential = resolve_target_credential(target, None, local_dev_token_fallback(target))?; let oauth_session = refreshable_oauth(target, credential.as_ref());