scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-10-09 03:20:56 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

fix(server): honor server.listen when bind is omitted

Browse source
This commit is contained in:
Bryan Helmkamp 2026-04-13 23:54:08 -04:00
parent ee5cd96f52
commit d22be78575
6 changed files with 227 additions and 47 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
docs/administration/server-configuration.mdx
View file

@ -102,7 +102,7 @@ Several `settings.toml` settings can be overridden via `fabro server start` flag
| Flag | Default | Description |
|---|---|---|
| `--bind` | `~/.fabro/fabro.sock` | Address to bind: `IP` or `IP:port` for TCP, or a path for Unix socket |
| `--bind` | Resolved `[server.listen]`, falling back to `~/.fabro/fabro.sock` when `[server.listen]` is absent | Address to bind: `IP` or `IP:port` for TCP, or a path for Unix socket |
| `--web` | enabled | Enable the embedded web UI, browser auth routes, and web-only helper endpoints |
| `--no-web` | disabled | Disable the embedded web UI, browser auth routes, and web-only helper endpoints |
| `--foreground` | — | Run in the foreground instead of daemonizing |

6
docs/reference/cli.mdx
View file

@ -329,10 +329,10 @@ fabro model test -m claude-sonnet-4-5
## `fabro server start`
Start the Fabro server daemon. By default, the server launches as a background process listening on a Unix socket. Use `--foreground` for the previous blocking behavior.
Start the Fabro server daemon. By default, the server launches as a background process using the resolved `[server.listen]` setting, and falls back to a Unix socket when `[server.listen]` is absent. Use `--foreground` for the previous blocking behavior.
```bash
fabro server start # background daemon on Unix socket
fabro server start # background daemon using server.listen or the default Unix socket
fabro server start --bind 127.0.0.1 # TCP on 32276, or random port if 32276 is busy
fabro server start --bind 127.0.0.1:8080 # TCP on a specific port
fabro server start --no-web # API and /health only
@ -342,7 +342,7 @@ fabro server start --sandbox daytona --max-concurrent-runs 4
| Flag | Description | Default |
|---|---|---|
| `--bind <ADDR>` | Address to bind: `IP` or `IP:port` for TCP, or a path for Unix socket | `~/.fabro/fabro.sock` |
| `--bind <ADDR>` | Address to bind: `IP` or `IP:port` for TCP, or a path for Unix socket | Resolved `[server.listen]`, falling back to `~/.fabro/fabro.sock` when `[server.listen]` is absent |
| `--web` | Enable the embedded web UI, browser auth routes, and web-only helper endpoints | Enabled |
| `--no-web` | Disable the embedded web UI, browser auth routes, and web-only helper endpoints | Disabled |
| `--foreground` | Run in the foreground instead of daemonizing | — |

23
lib/crates/fabro-cli/src/commands/server/mod.rs
View file

@ -7,9 +7,7 @@ pub(crate) mod stop;
use std::time::Duration;
use anyhow::Result;
use fabro_server::bind;
use fabro_server::bind::BindRequest;
use fabro_server::serve::ServeArgs;
use fabro_server::serve::{self, ServeArgs};
use fabro_util::printer::Printer;
use fabro_util::terminal::Styles;
@ -34,10 +32,8 @@ pub(crate) async fn dispatch(
storage_dir.as_deref(),
)?;
let storage_dir = user_config::storage_dir(&settings)?;
let bind_addr = match serve_args.bind.as_deref() {
Some(s) => bind::parse_bind(s)?,
None => BindRequest::Unix(user_config::default_socket_path()),
};
let bind_addr =
serve::resolve_bind_request_from_settings(&settings, serve_args.bind.as_deref())?;
let styles: &'static Styles = Box::leak(Box::new(Styles::detect_stderr()));
Box::pin(start::execute(
bind_addr,
@ -68,19 +64,18 @@ pub(crate) async fn dispatch(
record_path,
serve_args,
}) => {
let settings = user_config::load_settings_with_config_and_storage_dir(
serve_args.config.as_deref(),
storage_dir.as_deref(),
)?;
let active_config_path = Some(
serve_args
.config
.clone()
.unwrap_or_else(|| user_config::active_settings_path(None)),
);
let bind_addr = if let Some(s) = serve_args.bind.as_deref() {
bind::parse_bind(s)?
} else {
// __serve should always receive an explicit --bind from the parent,
// but fall back to the storage dir default if missing.
BindRequest::Unix(user_config::default_socket_path())
};
let bind_addr =
serve::resolve_bind_request_from_settings(&settings, serve_args.bind.as_deref())?;
let styles: &'static Styles = Box::leak(Box::new(Styles::detect_stderr()));
Box::pin(foreground::execute(
record_path,

61
lib/crates/fabro-cli/src/commands/server/start.rs
View file

@ -1,9 +1,9 @@
use std::path::{Path, PathBuf};
use std::time::Duration;
use anyhow::{Result, bail};
use anyhow::{Result, anyhow, bail};
use chrono::Utc;
use fabro_config::user::default_socket_path;
use fabro_config::user::load_settings_config;
use fabro_config::{Storage, envfile};
use fabro_server::bind::{Bind, BindRequest};
use fabro_server::serve;
@ -44,12 +44,7 @@ pub(crate) async fn ensure_server_running_for_storage(
storage_dir: &Path,
config_path: &Path,
) -> Result<Bind> {
if let Some(existing) = record::active_server_record(storage_dir) {
return Ok(existing.bind);
}
let bind = Bind::Unix(default_socket_path());
ensure_server_running_with_bind(bind, config_path, storage_dir).await
ensure_server_running_with_bind(None, config_path, storage_dir).await
}
pub(crate) async fn ensure_server_running_on_socket(
@ -57,18 +52,25 @@ pub(crate) async fn ensure_server_running_on_socket(
config_path: &Path,
storage_dir: &Path,
) -> Result<()> {
let bind = Bind::Unix(socket_path.to_path_buf());
let _ = ensure_server_running_with_bind(bind, config_path, storage_dir).await?;
let _ = ensure_server_running_with_bind(
Some(BindRequest::Unix(socket_path.to_path_buf())),
config_path,
storage_dir,
)
.await?;
Ok(())
}
async fn ensure_server_running_with_bind(
bind: Bind,
bind_request: Option<BindRequest>,
config_path: &Path,
storage_dir: &Path,
) -> Result<Bind> {
if let Some(existing) = record::active_server_record(storage_dir) {
if existing.bind == bind {
if bind_request
.as_ref()
.is_none_or(|requested| bind_matches_request(&existing.bind, requested))
{
return Ok(existing.bind);
}
bail!(
@ -79,7 +81,7 @@ async fn ensure_server_running_with_bind(
}
let serve_args = ServeArgs {
bind: None,
bind: bind_request.as_ref().map(ToString::to_string),
web: false,
no_web: false,
model: None,
@ -90,9 +92,12 @@ async fn ensure_server_running_with_bind(
config: Some(config_path.to_path_buf()),
};
let bind_request = match &bind {
Bind::Unix(path) => BindRequest::Unix(path.clone()),
Bind::Tcp(addr) => BindRequest::Tcp(*addr),
let bind_request = match bind_request {
Some(bind_request) => bind_request,
None => {
let settings = load_settings_config(Some(config_path))?;
serve::resolve_bind_request_from_settings(&settings, None)?
}
};
match execute_daemon(
@ -104,7 +109,14 @@ async fn ensure_server_running_with_bind(
)
.await
{
Ok(()) => Ok(bind),
Ok(()) => record::active_server_record(storage_dir)
.map(|server| server.bind)
.ok_or_else(|| {
anyhow!(
"Server started but no active record was found for {}",
storage_dir.display()
)
}),
Err(err) => {
if let Some(existing) = record::active_server_record(storage_dir) {
Ok(existing.bind)
@ -115,6 +127,21 @@ async fn ensure_server_running_with_bind(
}
}
fn bind_matches_request(existing: &Bind, requested: &BindRequest) -> bool {
match (existing, requested) {
(Bind::Unix(existing_path), BindRequest::Unix(requested_path)) => {
existing_path == requested_path
}
(Bind::Tcp(existing_addr), BindRequest::Tcp(requested_addr)) => {
existing_addr == requested_addr
}
(Bind::Tcp(existing_addr), BindRequest::TcpHost(requested_host)) => {
existing_addr.ip() == *requested_host
}
_ => false,
}
}
fn server_max_concurrent_runs_override() -> Option<usize> {
std::env::var("FABRO_SERVER_MAX_CONCURRENT_RUNS")
.ok()

65
lib/crates/fabro-cli/tests/it/cmd/server_start.rs
View file

@ -138,6 +138,71 @@ fn start_without_bind_uses_home_socket_instead_of_storage_socket() {
.success();
}
#[test]
fn start_without_bind_uses_configured_tcp_listen_address() {
let context = test_context!();
let storage_root = isolated_storage_dir();
let storage_dir = storage_root.path().join("storage");
let config_dir = tempfile::tempdir_in("/tmp").unwrap();
let config_path = config_dir.path().join("settings.toml");
std::fs::write(
&config_path,
r#"
_version = 1
[server.listen]
type = "tcp"
address = "127.0.0.1:0"
"#,
)
.unwrap();
let mut cmd = context.command();
cmd.env("FABRO_STORAGE_DIR", &storage_dir);
cmd.args([
"server",
"start",
"--dry-run",
"--config",
config_path.to_str().unwrap(),
]);
let output = cmd.output().expect("server start command should run");
assert!(
output.status.success(),
"server start should succeed: {}",
String::from_utf8_lossy(&output.stderr)
);
let stderr = String::from_utf8_lossy(&output.stderr);
let bind_regex = regex::Regex::new(r"127\.0\.0\.1:\d+").unwrap();
assert!(
bind_regex.is_match(&stderr),
"expected configured tcp bind in stderr, got {stderr}"
);
let output = context
.command()
.env("FABRO_STORAGE_DIR", &storage_dir)
.args(["server", "status", "--json"])
.assert()
.success()
.get_output()
.stdout
.clone();
let json: serde_json::Value = serde_json::from_slice(&output).unwrap();
let bind = json["bind"].as_str().expect("bind should be a string");
assert!(
bind.starts_with("127.0.0.1:"),
"expected configured tcp bind, got {bind}"
);
context
.command()
.env("FABRO_STORAGE_DIR", &storage_dir)
.args(["server", "stop"])
.assert()
.success();
}
#[test]
fn start_with_tcp_host_only_bind_resolves_to_host_and_port() {
let context = test_context!();

117
lib/crates/fabro-server/src/serve.rs
View file

@ -4,10 +4,11 @@ use std::time::Duration;
use anyhow::Context;
use clap::Args;
use fabro_config::merge::combine_files;
use fabro_config::user::load_settings_config;
use fabro_config::{Storage, resolve_server_from_file};
use fabro_sandbox::SandboxProvider;
use fabro_types::settings::server::GithubIntegrationStrategy;
use fabro_types::settings::server::{GithubIntegrationStrategy, ServerLayer, ServerListenLayer};
use fabro_types::settings::{
InterpString, ObjectStoreSettings, ServerListenSettings,
ServerSettings as ResolvedServerSettings, SettingsLayer,
@ -187,6 +188,51 @@ fn resolve_server_settings(file: &SettingsLayer) -> anyhow::Result<ResolvedServe
})
}
pub fn resolve_bind_request_from_settings(
settings: &SettingsLayer,
explicit_bind: Option<&str>,
) -> anyhow::Result<BindRequest> {
let effective_settings = match explicit_bind.map(bind::parse_bind).transpose()? {
Some(BindRequest::TcpHost(host)) => return Ok(BindRequest::TcpHost(host)),
Some(bind) => combine_files(settings.clone(), bind_override_layer(bind)),
None => settings.clone(),
};
let resolved = resolve_server_settings(&effective_settings)?;
resolved_bind_request(&resolved)
}
fn bind_override_layer(bind: BindRequest) -> SettingsLayer {
let listen = match bind {
BindRequest::Unix(path) => ServerListenLayer::Unix {
path: Some(InterpString::parse(&path.display().to_string())),
},
BindRequest::Tcp(address) => ServerListenLayer::Tcp {
address: Some(InterpString::parse(&address.to_string())),
tls: None,
},
BindRequest::TcpHost(_) => {
unreachable!("host-only bind requests are handled before building a settings override")
}
};
SettingsLayer {
server: Some(ServerLayer {
listen: Some(listen),
..ServerLayer::default()
}),
..SettingsLayer::default()
}
}
fn resolved_bind_request(
resolved_server_settings: &ResolvedServerSettings,
) -> anyhow::Result<BindRequest> {
match &resolved_server_settings.listen {
ServerListenSettings::Unix { path } => Ok(BindRequest::Unix(resolve_interp_path(path)?)),
ServerListenSettings::Tcp { address, .. } => Ok(BindRequest::Tcp(*address)),
}
}
fn resolve_interp(value: &InterpString) -> anyhow::Result<String> {
value
.resolve(|name| std::env::var(name).ok())
@ -294,6 +340,8 @@ where
// Shared config for live reloading
let effective_settings = apply_runtime_settings(&disk_settings, &args, dry_run_mode, &data_dir);
let resolved_server_settings = resolve_server_settings(&effective_settings)?;
let bind_request =
resolve_bind_request_from_settings(&effective_settings, args.bind.as_deref())?;
let shared_settings = Arc::new(RwLock::new(effective_settings));
std::fs::create_dir_all(&data_dir)?;
let (auth_mode, max_concurrent_runs) = {
@ -337,14 +385,6 @@ where
let router =
build_router_with_options(Arc::clone(&state), auth_mode, RouterOptions { web_enabled });
let bind_request = match args.bind {
Some(ref s) => bind::parse_bind(s)?,
None => match &resolved_server_settings.listen {
ServerListenSettings::Unix { path } => BindRequest::Unix(resolve_interp_path(path)?),
ServerListenSettings::Tcp { address, .. } => BindRequest::Tcp(*address),
},
};
// Optionally start webhook listener
let webhook_manager = match resolved_server_settings.integrations.github.strategy {
GithubIntegrationStrategy::Token => None,
@ -663,13 +703,14 @@ mod tests {
use fabro_config::parse_settings_layer;
use fabro_types::settings::SettingsLayer;
use fabro_util::Home;
use super::{
ServeArgs, ServerTitlePhase, apply_runtime_settings, bind_tcp_host_with_fallback,
build_object_store_with_preference, resolve_server_settings, router_web_enabled,
server_bind_title, server_title,
build_object_store_with_preference, resolve_bind_request_from_settings,
resolve_server_settings, router_web_enabled, server_bind_title, server_title,
};
use crate::bind::Bind;
use crate::bind::{Bind, BindRequest};
fn parse_settings(source: &str) -> SettingsLayer {
parse_settings_layer(source).expect("v2 fixture should parse")
@ -763,6 +804,58 @@ enabled = false
);
}
#[test]
fn resolve_bind_request_from_settings_defaults_to_socket_when_listen_is_absent() {
let bind =
resolve_bind_request_from_settings(&SettingsLayer::default(), None).expect("bind");
assert_eq!(bind, BindRequest::Unix(Home::from_env().socket_path()));
}
#[test]
fn resolve_bind_request_from_settings_uses_configured_tcp_when_no_explicit_bind_is_given() {
let settings = parse_settings(
r#"
_version = 1
[server.listen]
type = "tcp"
address = "127.0.0.1:0"
"#,
);
let bind = resolve_bind_request_from_settings(&settings, None).expect("bind");
assert_eq!(bind, BindRequest::Tcp("127.0.0.1:0".parse().unwrap()));
}
#[test]
fn resolve_bind_request_from_settings_prefers_explicit_bind_over_config() {
let settings = parse_settings(
r#"
_version = 1
[server.listen]
type = "tcp"
address = "127.0.0.1:32276"
"#,
);
let bind =
resolve_bind_request_from_settings(&settings, Some("/tmp/fabro.sock")).expect("bind");
assert_eq!(bind, BindRequest::Unix(PathBuf::from("/tmp/fabro.sock")));
}
#[test]
fn resolve_bind_request_from_settings_preserves_host_only_cli_bind() {
let settings = SettingsLayer::default();
let bind = resolve_bind_request_from_settings(&settings, Some("127.0.0.1")).expect("bind");
assert_eq!(bind, BindRequest::TcpHost("127.0.0.1".parse().unwrap()));
}
#[test]
fn web_enabled_stays_enabled_without_github_app_mode() {
let base = parse_settings(