fix settings runtime refresh follow-ups

This commit is contained in:
Bryan Helmkamp 2026-04-22 19:44:20 -04:00
parent ebb8bf7add
commit bb0d05be2b
No known key found for this signature in database
6 changed files with 121 additions and 12 deletions

View file

@ -3,8 +3,14 @@ set -euo pipefail
cd "$(dirname "$0")/../.."
symbol_allowlist=(
server_symbol_allowlist=(
"lib/crates/fabro-cli/src/local_server.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"
)
storage_allowlist=(
"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"
@ -50,11 +56,19 @@ fail=0
while IFS= read -r path; do
[[ -z "$path" ]] && continue
if ! in_array "$path" "${symbol_allowlist[@]}"; then
if ! in_array "$path" "${server_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|fabro_config::ServerSettings::from_layer\b|fabro_config::ServerSettings::resolve\b|ServerSettings::from_layer\b|ServerSettings::resolve\b|Storage::new')
done < <(find_matches 'fabro_config::resolve_server_from_file|fabro_config::resolve_server\b|fabro_config::ServerSettings::from_layer\b|fabro_config::ServerSettings::resolve\b|ServerSettings::from_layer\b|ServerSettings::resolve\b')
while IFS= read -r path; do
[[ -z "$path" ]] && continue
if ! in_array "$path" "${storage_allowlist[@]}"; then
echo "boundary check failed: Storage::new used outside allowlist: $path" >&2
fail=1
fi
done < <(find_matches 'Storage::new')
while IFS= read -r path; do
[[ -z "$path" ]] && continue

View file

@ -1277,8 +1277,7 @@ async fn write_artifact_store_metadata(
settings: &SettingsLayer,
fabro_version: &str,
) -> Result<()> {
let resolved =
fabro_config::ServerSettings::from_layer(settings).map_err(anyhow::Error::from)?;
let resolved = local_server::server_settings(settings)?;
let (object_store, prefix) = serve::build_artifact_object_store(&resolved.server)?;
let artifact_store = ArtifactStore::new(object_store, prefix);
artifact_store.write_metadata(fabro_version).await?;
@ -1797,7 +1796,7 @@ async fn run_install_inner(
.context("failed to parse generated settings.toml")?,
args.storage_dir.as_deref(),
);
fabro_config::ServerSettings::from_layer(&install_settings).map_err(anyhow::Error::from)?;
local_server::server_settings(&install_settings)?;
// Secrets and auth material
{

View file

@ -26,8 +26,12 @@ pub(crate) fn bind_request(
resolve_bind_request_from_settings(settings, cli_override)
}
pub(crate) fn server_settings(settings: &SettingsLayer) -> Result<fabro_config::ServerSettings> {
fabro_config::ServerSettings::from_layer(settings).map_err(anyhow::Error::from)
}
pub(crate) fn auth_methods(settings: &SettingsLayer) -> Vec<ServerAuthMethod> {
fabro_config::ServerSettings::from_layer(settings)
server_settings(settings)
.map(|resolved| resolved.server.auth.methods)
.unwrap_or_default()
}

View file

@ -134,16 +134,12 @@ fn help_smoke_covers_high_cost_commands() {
----- stdout -----
Inspect effective settings
Usage: fabro settings [OPTIONS] [WORKFLOW]
Arguments:
[WORKFLOW] Optional workflow name, .fabro path, or .toml run config to overlay
Usage: fabro settings [OPTIONS]
Options:
--json Output as JSON [env: FABRO_JSON=]
--server <SERVER> Fabro server target: http(s) URL or absolute Unix socket path [env: FABRO_SERVER=]
--debug Enable DEBUG-level logging (default is INFO) [env: FABRO_DEBUG=]
--local Show only locally resolved settings and skip the server call
--no-upgrade-check Disable automatic upgrade check [env: FABRO_NO_UPGRADE_CHECK=true]
--quiet Suppress non-essential output [env: FABRO_QUIET=]
--verbose Enable verbose output [env: FABRO_VERBOSE=]

View file

@ -901,6 +901,7 @@ mod tests {
resolve_server_settings, resolve_startup_github_webhook_ip_allowlist, router_web_enabled,
server_bind_title, server_title,
};
use crate::server::create_app_state_with_options;
fn parse_settings(source: &str) -> SettingsLayer {
let mut layer = parse_settings_layer(source).expect("v2 fixture should parse");
@ -935,6 +936,42 @@ mod tests {
assert_eq!(storage_root.as_deref(), Some("/srv/fabro-storage"));
}
#[test]
fn app_state_server_settings_use_effective_runtime_layer_storage_override() {
let base = parse_settings(
r#"
_version = 1
[server.storage]
root = "/srv/from-disk"
"#,
);
let args = ServeArgs {
bind: None,
model: None,
provider: None,
sandbox: None,
web: false,
no_web: false,
max_concurrent_runs: None,
config: None,
#[cfg(debug_assertions)]
watch_web: false,
};
let effective = apply_runtime_settings(&base, &args, &PathBuf::from("/srv/from-runtime"));
let state = create_app_state_with_options(effective, 5);
assert_eq!(
state.server_settings().server.storage.root.as_source(),
"/srv/from-runtime"
);
assert_eq!(
state.server_storage_dir(),
PathBuf::from("/srv/from-runtime")
);
}
#[test]
fn apply_runtime_settings_enables_web_from_cli_flag() {
let base = parse_settings(

View file

@ -7397,6 +7397,65 @@ url = "{url}"
}
}
#[test]
fn replace_settings_updates_layer_and_typed_server_settings() {
let state = create_app_state_with_options(
fabro_config::parse_settings_layer(
r#"
_version = 1
[server.auth]
methods = ["dev-token"]
[server.web]
url = "http://old.example.com"
[server.storage]
root = "/srv/old"
"#,
)
.expect("settings fixture should parse"),
5,
);
let updated = fabro_config::parse_settings_layer(
r#"
_version = 1
[server.auth]
methods = ["dev-token"]
[server.web]
url = "http://new.example.com"
[server.storage]
root = "/srv/new"
"#,
)
.expect("settings fixture should parse");
state
.replace_settings(updated)
.expect("valid settings should replace current state");
assert_eq!(state.canonical_origin().unwrap(), "http://new.example.com");
assert_eq!(
state.server_settings().server.storage.root.as_source(),
"/srv/new"
);
let layer_root = state
.settings
.read()
.expect("settings lock poisoned")
.server
.as_ref()
.and_then(|server| server.storage.as_ref())
.and_then(|storage| storage.root.as_ref())
.map(InterpString::as_source);
assert_eq!(layer_root.as_deref(), Some("/srv/new"));
}
#[tokio::test]
async fn create_secret_stores_file_secret_and_excludes_it_from_snapshot() {
let state = create_app_state();