mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-08-28 05:27:41 +00:00
feat(workflow): support overriding cwd for local sandbox provider (#467)
Some checks failed
Rust / Format (push) Has been cancelled
Rust / Clippy (push) Has been cancelled
Rust / Generated Docs (push) Has been cancelled
TypeScript / Build (push) Has been cancelled
Rust / Test (Linux) (push) Has been cancelled
Rust / Test (macOS) (push) Has been cancelled
TypeScript / Typecheck (push) Has been cancelled
TypeScript / Test (push) Has been cancelled
Some checks failed
Rust / Format (push) Has been cancelled
Rust / Clippy (push) Has been cancelled
Rust / Generated Docs (push) Has been cancelled
TypeScript / Build (push) Has been cancelled
Rust / Test (Linux) (push) Has been cancelled
Rust / Test (macOS) (push) Has been cancelled
TypeScript / Typecheck (push) Has been cancelled
TypeScript / Test (push) Has been cancelled
## Problem The `local` sandbox uses the run's `source_directory` (the CLI's cwd at invocation time) as its working directory and `create_dir_all`s it on the server (`LocalSandbox::initialize` in `fabro-sandbox`). That is correct when the CLI and the server share a host — the agent operates directly on the user's project tree. When the server is **remote** from the CLI — e.g. `fabro serve` running in a container in Kubernetes, driven over HTTP with the `local` sandbox — the client's cwd (e.g. `/Users/alice/project`) does not exist on the server. The sandbox then tries to create that path as the (often unprivileged) server user and fails at init: ``` sandbox.failed provider="local" error="Failed to create working directory" causes=["Permission denied (os error 13)"] ``` and the run dies with `workflow_error` before the agent starts. ## Fix When `source_directory` is absent or does not exist on the server, fall back to a server-writable `workspace` directory under the run's scratch dir instead of recreating the client path. **Same-host behavior is unchanged**: an existing `source_directory` is still used as-is. The selection is extracted into a small pure helper, `local_working_directory(source_directory, run_dir)`, so it can be unit-tested directly. ## Testing - `cargo test -p fabro-workflow local_working_directory` — 3 new tests (existing source dir → used; absent → fallback; present-but-missing-on-server → fallback) - `cargo check -p fabro-workflow` 🤖 Generated with [Claude Code](https://claude.com/claude-code) Thanks for fabro @brynary! --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
bc0bda73a6
commit
a769336c39
20 changed files with 516 additions and 25 deletions
|
|
@ -6371,6 +6371,10 @@ components:
|
|||
example: 0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef
|
||||
provider:
|
||||
$ref: "#/components/schemas/EnvironmentProvider"
|
||||
cwd:
|
||||
type: ["string", "null"]
|
||||
description: Local-provider command working directory for this environment. Docker and Daytona ignore this value.
|
||||
example: /srv/fabro/workspaces/team-a
|
||||
image:
|
||||
$ref: "#/components/schemas/EnvironmentApiImageSettings"
|
||||
resources:
|
||||
|
|
@ -6406,6 +6410,10 @@ components:
|
|||
example: docker
|
||||
provider:
|
||||
$ref: "#/components/schemas/EnvironmentProvider"
|
||||
cwd:
|
||||
type: ["string", "null"]
|
||||
description: Local-provider command working directory for this environment. Docker and Daytona ignore this value.
|
||||
example: /srv/fabro/workspaces/team-a
|
||||
image:
|
||||
$ref: "#/components/schemas/EnvironmentApiImageSettings"
|
||||
resources:
|
||||
|
|
@ -6436,6 +6444,10 @@ components:
|
|||
properties:
|
||||
provider:
|
||||
$ref: "#/components/schemas/EnvironmentProvider"
|
||||
cwd:
|
||||
type: ["string", "null"]
|
||||
description: Local-provider command working directory for this environment. Docker and Daytona ignore this value.
|
||||
example: /srv/fabro/workspaces/team-a
|
||||
image:
|
||||
$ref: "#/components/schemas/EnvironmentApiImageSettings"
|
||||
resources:
|
||||
|
|
@ -13342,6 +13354,10 @@ components:
|
|||
type: string
|
||||
provider:
|
||||
$ref: "#/components/schemas/EnvironmentProvider"
|
||||
cwd:
|
||||
type: ["string", "null"]
|
||||
description: Local-provider command working directory for this environment. Docker and Daytona ignore this value.
|
||||
example: /srv/fabro/workspaces/team-a
|
||||
image:
|
||||
$ref: "#/components/schemas/EnvironmentImageSettings"
|
||||
resources:
|
||||
|
|
@ -13363,6 +13379,10 @@ components:
|
|||
properties:
|
||||
provider:
|
||||
$ref: "#/components/schemas/EnvironmentProvider"
|
||||
cwd:
|
||||
type: ["string", "null"]
|
||||
description: Local-provider command working directory for this environment. Docker and Daytona ignore this value.
|
||||
example: /srv/fabro/workspaces/team-a
|
||||
image:
|
||||
$ref: "#/components/schemas/EnvironmentImageSettings"
|
||||
resources:
|
||||
|
|
|
|||
|
|
@ -56,6 +56,21 @@ repo = "fabro-sh/fabro"
|
|||
NODE_ENV = "development"
|
||||
```
|
||||
|
||||
Server-managed local environments can also set `cwd`, an optional runtime
|
||||
command working directory:
|
||||
|
||||
```toml title="environments/host.toml"
|
||||
provider = "local"
|
||||
cwd = "/srv/fabro/workspaces/team-a"
|
||||
```
|
||||
|
||||
`cwd` is owned by the server environment and is only honored by the `local`
|
||||
provider. It is not a replacement for `run.working_dir`. Docker and Daytona
|
||||
ignore `cwd` and report a preflight warning because those clone-based providers
|
||||
own their workspace layout. Workflow, project, user, and direct-run
|
||||
`[environments.<slug>]` catalogs cannot set `cwd`; configure it in the
|
||||
server-managed environment file or through the environments API.
|
||||
|
||||
The same fields nest under `[environments.<slug>]` when defined in workflow or project TOML instead:
|
||||
|
||||
```toml title="workflow.toml"
|
||||
|
|
@ -150,6 +165,7 @@ stop_on_terminal = true
|
|||
| `labels` | Warning; ignored | Warning; ignored | Daytona labels |
|
||||
| `lifecycle.auto_stop` | Warning; ignored | Warning; ignored | Daytona auto-stop |
|
||||
| `env` | Process environment overlay | Container environment | Sandbox environment |
|
||||
| `cwd` | Server-side command working directory | Warning; ignored | Warning; ignored |
|
||||
|
||||
## Local
|
||||
|
||||
|
|
@ -157,8 +173,15 @@ stop_on_terminal = true
|
|||
|
||||
```toml title="environments/host.toml"
|
||||
provider = "local"
|
||||
cwd = "/srv/fabro/workspaces/team-a"
|
||||
```
|
||||
|
||||
When `cwd` is set, local runs execute commands from that absolute server-side
|
||||
path. When it is unset, Fabro keeps same-host compatibility by using the
|
||||
submitted source directory only if that path exists on the server. If neither is
|
||||
available, the run fails before execution with a remediation to configure
|
||||
`cwd`.
|
||||
|
||||
Fabro hard-errors if a local environment asks for blocked or CIDR-restricted networking because the provider cannot enforce it.
|
||||
|
||||
## Docker
|
||||
|
|
@ -179,7 +202,7 @@ memory = "4GB"
|
|||
mode = "block"
|
||||
```
|
||||
|
||||
Docker and Daytona are clone-based providers. When a run has a GitHub origin, Fabro clones it into the provider workspace. Set `[run.clone] enabled = false` to start with an empty workspace.
|
||||
Docker and Daytona are clone-based providers. When a run has a GitHub origin, Fabro clones it into the provider workspace. Set `[run.clone] enabled = false` to start with an empty workspace. Docker and Daytona ignore `cwd`; use the provider-owned workspace layout and `run.working_dir` for repository-relative commands.
|
||||
|
||||
## Daytona
|
||||
|
||||
|
|
|
|||
|
|
@ -50,6 +50,17 @@ fn environment_response_round_trips_public_json_shape() {
|
|||
assert_eq!(serde_json::to_value(api).unwrap(), value);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn environment_response_round_trips_cwd_json_shape() {
|
||||
let mut value = environment_settings_json();
|
||||
value["id"] = json!("docker-inline");
|
||||
value["revision"] = json!("0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef");
|
||||
value["cwd"] = json!("/workspace/custom");
|
||||
|
||||
let api: ApiEnvironment = serde_json::from_value(value.clone()).unwrap();
|
||||
assert_eq!(serde_json::to_value(api).unwrap(), value);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn create_environment_request_round_trips_inline_dockerfile_json_shape() {
|
||||
let mut value = environment_settings_json();
|
||||
|
|
|
|||
|
|
@ -12,6 +12,8 @@ pub struct EnvironmentLayer {
|
|||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub provider: Option<String>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub cwd: Option<String>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub image: Option<EnvironmentImageLayer>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub resources: Option<EnvironmentResourcesLayer>,
|
||||
|
|
@ -49,6 +51,7 @@ impl RunEnvironmentLayer {
|
|||
pub fn into_environment_override(self) -> EnvironmentLayer {
|
||||
EnvironmentLayer {
|
||||
provider: None,
|
||||
cwd: None,
|
||||
image: self.image,
|
||||
resources: self.resources,
|
||||
network: self.network,
|
||||
|
|
|
|||
|
|
@ -31,7 +31,14 @@ const LEGACY_LLM_KEYS: &[&str] = &[
|
|||
pub enum ParseError {
|
||||
Toml(String),
|
||||
Version(VersionError),
|
||||
UnknownTopLevelKey { key: String, hint: Option<String> },
|
||||
UnknownTopLevelKey {
|
||||
key: String,
|
||||
hint: Option<String>,
|
||||
},
|
||||
ServerManagedEnvironmentCwd {
|
||||
path: String,
|
||||
source: SettingsSource,
|
||||
},
|
||||
}
|
||||
|
||||
impl fmt::Display for ParseError {
|
||||
|
|
@ -49,6 +56,10 @@ impl fmt::Display for ParseError {
|
|||
)
|
||||
}
|
||||
}
|
||||
Self::ServerManagedEnvironmentCwd { path, source } => write!(
|
||||
f,
|
||||
"`{path}` is server-managed and cannot be set in {source} settings; configure cwd on a server-managed environment instead."
|
||||
),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -128,12 +139,40 @@ impl SettingsSource {
|
|||
pub(crate) fn runs_settings_migrations(self) -> bool {
|
||||
matches!(self, Self::ActiveSettings)
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
fn allows_environment_cwd(self) -> bool {
|
||||
matches!(self, Self::ActiveSettings)
|
||||
}
|
||||
}
|
||||
|
||||
impl fmt::Display for SettingsSource {
|
||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
let label = match self {
|
||||
Self::ActiveSettings => "active server",
|
||||
Self::Project => "project",
|
||||
Self::Workflow => "workflow",
|
||||
Self::DirectRun => "direct-run",
|
||||
Self::User => "user",
|
||||
};
|
||||
f.write_str(label)
|
||||
}
|
||||
}
|
||||
|
||||
pub fn validate_settings_source(
|
||||
_layer: &SettingsLayer,
|
||||
_source: SettingsSource,
|
||||
layer: &SettingsLayer,
|
||||
source: SettingsSource,
|
||||
) -> Result<(), ParseError> {
|
||||
if !source.allows_environment_cwd() {
|
||||
for (id, environment) in layer.environments.iter() {
|
||||
if environment.cwd.is_some() {
|
||||
return Err(ParseError::ServerManagedEnvironmentCwd {
|
||||
path: format!("environments.{id}.cwd"),
|
||||
source,
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,3 +1,5 @@
|
|||
use std::path::Path;
|
||||
|
||||
use fabro_types::settings::run::{
|
||||
DockerfileSource, EnvironmentImageSettings, EnvironmentLifecycleSettings,
|
||||
EnvironmentNetworkMode, EnvironmentNetworkSettings, EnvironmentProvider,
|
||||
|
|
@ -71,6 +73,7 @@ fn resolve_environment_fields(
|
|||
|
||||
let environment = EnvironmentSettings {
|
||||
provider,
|
||||
cwd: resolve_cwd(layer.cwd.as_deref(), &format!("{path}.cwd"), errors),
|
||||
image: resolve_image(layer.image.as_ref()),
|
||||
resources: resolve_resources(layer.resources.as_ref()),
|
||||
network: resolve_network(layer.network.as_ref(), &format!("{path}.network"), errors),
|
||||
|
|
@ -82,6 +85,25 @@ fn resolve_environment_fields(
|
|||
environment
|
||||
}
|
||||
|
||||
fn resolve_cwd(raw: Option<&str>, path: &str, errors: &mut Vec<ResolveError>) -> Option<String> {
|
||||
let raw = raw?;
|
||||
if raw.trim().is_empty() {
|
||||
errors.push(ResolveError::Invalid {
|
||||
path: path.to_string(),
|
||||
reason: "cwd must not be empty".to_string(),
|
||||
});
|
||||
return None;
|
||||
}
|
||||
if !Path::new(raw).is_absolute() {
|
||||
errors.push(ResolveError::Invalid {
|
||||
path: path.to_string(),
|
||||
reason: "cwd must be an absolute path".to_string(),
|
||||
});
|
||||
return None;
|
||||
}
|
||||
Some(raw.to_string())
|
||||
}
|
||||
|
||||
fn parse_provider(raw: &str, path: &str, errors: &mut Vec<ResolveError>) -> EnvironmentProvider {
|
||||
if let Ok(provider) = raw.parse::<EnvironmentProvider>() {
|
||||
provider
|
||||
|
|
|
|||
|
|
@ -179,6 +179,76 @@ NODE_ENV = "development"
|
|||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolves_environment_cwd_from_injected_server_catalog() {
|
||||
let settings = workflow_settings_from_toml_with_catalog(
|
||||
r#"
|
||||
_version = 1
|
||||
|
||||
[run.environment]
|
||||
id = "host"
|
||||
"#,
|
||||
r#"
|
||||
[environments.host]
|
||||
provider = "local"
|
||||
cwd = "/srv/fabro/workspaces/team-a"
|
||||
"#,
|
||||
)
|
||||
.expect("server-managed environment cwd should resolve");
|
||||
|
||||
assert_eq!(
|
||||
settings.run.environment.cwd.as_deref(),
|
||||
Some("/srv/fabro/workspaces/team-a")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_environment_cwd_in_client_workflow_catalog() {
|
||||
let err = workflow_settings_from_toml(
|
||||
r#"
|
||||
_version = 1
|
||||
|
||||
[run.environment]
|
||||
id = "host"
|
||||
|
||||
[environments.host]
|
||||
provider = "local"
|
||||
cwd = "/srv/fabro/workspaces/team-a"
|
||||
"#,
|
||||
)
|
||||
.expect_err("client-owned workflow environments must not set cwd");
|
||||
|
||||
let message = err.to_string();
|
||||
assert!(
|
||||
message.contains("environments.host.cwd") && message.contains("server-managed"),
|
||||
"unexpected error: {message}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_relative_environment_cwd_from_server_catalog() {
|
||||
let err = workflow_settings_from_toml_with_catalog(
|
||||
r#"
|
||||
_version = 1
|
||||
|
||||
[run.environment]
|
||||
id = "host"
|
||||
"#,
|
||||
r#"
|
||||
[environments.host]
|
||||
provider = "local"
|
||||
cwd = "relative/workspace"
|
||||
"#,
|
||||
)
|
||||
.expect_err("relative environment cwd should not resolve");
|
||||
|
||||
let message = err.to_string();
|
||||
assert!(
|
||||
message.contains("environment.cwd") && message.contains("absolute path"),
|
||||
"unexpected error: {message}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolves_run_level_clone_branch_controls() {
|
||||
let settings = super::workflow_settings_from_toml(
|
||||
|
|
|
|||
|
|
@ -101,6 +101,9 @@ pub(crate) fn canonical_bytes(layer: &EnvironmentLayer) -> String {
|
|||
if let Some(provider) = layer.provider.as_deref() {
|
||||
doc["provider"] = value(provider);
|
||||
}
|
||||
if let Some(cwd) = layer.cwd.as_deref() {
|
||||
doc["cwd"] = value(cwd);
|
||||
}
|
||||
if let Some(image) = layer.image.as_ref() {
|
||||
append_image(doc.as_table_mut(), image);
|
||||
}
|
||||
|
|
@ -180,6 +183,7 @@ async fn inline_dense_dockerfile(
|
|||
fn environment_settings_to_layer(settings: &EnvironmentSettings) -> EnvironmentLayer {
|
||||
EnvironmentLayer {
|
||||
provider: Some(settings.provider.to_string()),
|
||||
cwd: settings.cwd.clone(),
|
||||
image: image_settings_to_layer(&settings.image),
|
||||
resources: resources_settings_to_layer(&settings.resources),
|
||||
network: network_settings_to_layer(&settings.network),
|
||||
|
|
|
|||
|
|
@ -479,6 +479,7 @@ mod tests {
|
|||
fn settings(provider: EnvironmentProvider) -> EnvironmentSettings {
|
||||
EnvironmentSettings {
|
||||
provider,
|
||||
cwd: None,
|
||||
image: EnvironmentImageSettings::default(),
|
||||
resources: EnvironmentResourcesSettings::default(),
|
||||
network: EnvironmentNetworkSettings::default(),
|
||||
|
|
@ -840,6 +841,86 @@ path = "Dockerfile"
|
|||
assert_ne!(created.revision, replaced.revision);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn create_persists_cwd_and_load_round_trips() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let environment_dir = dir.path().join("environments");
|
||||
let store = EnvironmentStore::load(&environment_dir, true).unwrap();
|
||||
let mut settings = settings(EnvironmentProvider::Local);
|
||||
settings.cwd = Some("/srv/fabro/workspaces/team-a".to_string());
|
||||
|
||||
let created = store
|
||||
.create(EnvironmentDraft {
|
||||
id: EnvironmentId::new("host").unwrap(),
|
||||
settings,
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
created.settings.cwd.as_deref(),
|
||||
Some("/srv/fabro/workspaces/team-a")
|
||||
);
|
||||
let persisted = fs::read_to_string(environment_dir.join("host.toml"))
|
||||
.await
|
||||
.unwrap();
|
||||
assert!(persisted.contains("cwd = \"/srv/fabro/workspaces/team-a\""));
|
||||
|
||||
let loaded = EnvironmentStore::load(&environment_dir, true).unwrap();
|
||||
let host = loaded.get(&EnvironmentId::new("host").unwrap()).unwrap();
|
||||
assert_eq!(
|
||||
host.settings.cwd.as_deref(),
|
||||
Some("/srv/fabro/workspaces/team-a")
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn create_rejects_relative_cwd() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let store = EnvironmentStore::load(dir.path().join("environments"), true).unwrap();
|
||||
let mut settings = settings(EnvironmentProvider::Local);
|
||||
settings.cwd = Some("relative/workspace".to_string());
|
||||
|
||||
let err = store
|
||||
.create(EnvironmentDraft {
|
||||
id: EnvironmentId::new("host").unwrap(),
|
||||
settings,
|
||||
})
|
||||
.await
|
||||
.unwrap_err();
|
||||
|
||||
assert!(matches!(err, EnvironmentStoreError::Validation { .. }));
|
||||
let message = err.to_string();
|
||||
assert!(
|
||||
message.contains("environment.cwd") && message.contains("absolute path"),
|
||||
"unexpected error: {message}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn load_rejects_empty_cwd() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let environment_dir = dir.path().join("environments");
|
||||
std::fs::create_dir_all(&environment_dir).unwrap();
|
||||
std::fs::write(
|
||||
environment_dir.join("bad.toml"),
|
||||
r#"
|
||||
provider = "local"
|
||||
cwd = ""
|
||||
"#,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let err = EnvironmentStore::load(&environment_dir, true).unwrap_err();
|
||||
|
||||
assert!(matches!(err, EnvironmentStoreError::Validation { .. }));
|
||||
let message = err.to_string();
|
||||
assert!(
|
||||
message.contains("environment.cwd") && message.contains("must not be empty"),
|
||||
"unexpected error: {message}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn api_dockerfile_path_is_resolved_relative_to_settings_dir_and_persisted_inline() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
|
|
|
|||
|
|
@ -3,6 +3,8 @@
|
|||
//! These mappings are consumed by both the workflow run-start path and the
|
||||
//! server preflight path, so they live here next to their destination types.
|
||||
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
#[cfg(feature = "daytona")]
|
||||
use fabro_types::settings::run::DockerfileSource as ResolvedDockerfileSource;
|
||||
use fabro_types::settings::run::{EnvironmentNetworkMode, RunEnvironmentSettings};
|
||||
|
|
@ -102,6 +104,30 @@ pub fn docker_config_from_environment(
|
|||
}
|
||||
}
|
||||
|
||||
pub fn local_working_directory_from_environment(
|
||||
settings: &RunEnvironmentSettings,
|
||||
source_directory: Option<&Path>,
|
||||
) -> crate::Result<PathBuf> {
|
||||
if let Some(cwd) = settings.cwd.as_deref() {
|
||||
return Ok(PathBuf::from(cwd));
|
||||
}
|
||||
|
||||
let Some(source_directory) = source_directory else {
|
||||
return Err(crate::Error::message(
|
||||
"local environment requires a server-side working directory; configure `environment.cwd = \"/absolute/path\"` on the selected local environment",
|
||||
));
|
||||
};
|
||||
|
||||
if source_directory.is_dir() {
|
||||
return Ok(source_directory.to_path_buf());
|
||||
}
|
||||
|
||||
Err(crate::Error::message(format!(
|
||||
"local environment source_directory does not exist or is not a directory on this server: {}. Configure `environment.cwd = \"/absolute/path\"` on the selected local environment for remote client/server deployments.",
|
||||
source_directory.display()
|
||||
)))
|
||||
}
|
||||
|
||||
#[cfg(feature = "docker")]
|
||||
#[expect(
|
||||
clippy::disallowed_methods,
|
||||
|
|
@ -122,3 +148,71 @@ fn size_to_gb_i32(bytes: u64) -> i32 {
|
|||
let gb = bytes / 1_000_000_000;
|
||||
i32::try_from(gb).unwrap_or(i32::MAX)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::collections::HashMap;
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
use fabro_types::settings::run::{
|
||||
EnvironmentImageSettings, EnvironmentLifecycleSettings, EnvironmentNetworkSettings,
|
||||
EnvironmentProvider, EnvironmentResourcesSettings,
|
||||
};
|
||||
|
||||
use super::*;
|
||||
|
||||
fn run_environment(provider: EnvironmentProvider) -> RunEnvironmentSettings {
|
||||
RunEnvironmentSettings {
|
||||
id: "host".to_string(),
|
||||
provider,
|
||||
cwd: None,
|
||||
image: EnvironmentImageSettings::default(),
|
||||
resources: EnvironmentResourcesSettings::default(),
|
||||
network: EnvironmentNetworkSettings::default(),
|
||||
lifecycle: EnvironmentLifecycleSettings::default(),
|
||||
labels: HashMap::new(),
|
||||
env: HashMap::new(),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn local_working_directory_prefers_environment_cwd() {
|
||||
let mut settings = run_environment(EnvironmentProvider::Local);
|
||||
settings.cwd = Some("/srv/fabro/workspaces/team-a".to_string());
|
||||
let missing_source = Path::new("/path/that/should/not/exist");
|
||||
|
||||
let resolved = local_working_directory_from_environment(&settings, Some(missing_source))
|
||||
.expect("configured cwd should be accepted");
|
||||
|
||||
assert_eq!(resolved, PathBuf::from("/srv/fabro/workspaces/team-a"));
|
||||
assert!(!missing_source.exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn local_working_directory_uses_existing_source_directory_without_cwd() {
|
||||
let settings = run_environment(EnvironmentProvider::Local);
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
|
||||
let resolved = local_working_directory_from_environment(&settings, Some(dir.path()))
|
||||
.expect("existing source directory should be accepted");
|
||||
|
||||
assert_eq!(resolved, dir.path());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn local_working_directory_rejects_missing_source_directory_without_cwd() {
|
||||
let settings = run_environment(EnvironmentProvider::Local);
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let missing = dir.path().join("client-only");
|
||||
|
||||
let err = local_working_directory_from_environment(&settings, Some(&missing))
|
||||
.expect_err("missing source directory without cwd should fail");
|
||||
|
||||
let message = err.to_string();
|
||||
assert!(
|
||||
message.contains("environment.cwd") && message.contains("does not exist"),
|
||||
"unexpected error: {message}"
|
||||
);
|
||||
assert!(!missing.exists());
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@ use fabro_model::{Catalog, ProviderId};
|
|||
use fabro_sandbox::daytona::DaytonaConfig;
|
||||
use fabro_sandbox::from_environment::{
|
||||
daytona_config_from_environment, docker_config_from_environment,
|
||||
local_working_directory_from_environment,
|
||||
};
|
||||
use fabro_sandbox::redact::redact_auth_url;
|
||||
use fabro_sandbox::{DockerSandboxOptions, Sandbox, SandboxSpec};
|
||||
|
|
@ -683,6 +684,9 @@ fn environment_capability_warnings(resolved_run: &RunNamespace) -> Vec<String> {
|
|||
}
|
||||
}
|
||||
EnvironmentProvider::Docker => {
|
||||
if environment.cwd.is_some() {
|
||||
warnings.push("docker provider ignores cwd".to_string());
|
||||
}
|
||||
if environment.resources.disk.is_some() {
|
||||
warnings.push("docker provider ignores disk resource limits".to_string());
|
||||
}
|
||||
|
|
@ -696,7 +700,11 @@ fn environment_capability_warnings(resolved_run: &RunNamespace) -> Vec<String> {
|
|||
warnings.push("docker provider ignores image.dockerfile".to_string());
|
||||
}
|
||||
}
|
||||
EnvironmentProvider::Daytona => {}
|
||||
EnvironmentProvider::Daytona => {
|
||||
if environment.cwd.is_some() {
|
||||
warnings.push("daytona provider ignores cwd".to_string());
|
||||
}
|
||||
}
|
||||
}
|
||||
warnings
|
||||
}
|
||||
|
|
@ -850,17 +858,21 @@ fn preflight_sandbox_spec(
|
|||
resolved_run: &RunNamespace,
|
||||
github_app: Option<fabro_github::GitHubCredentials>,
|
||||
daytona_api_key: Option<String>,
|
||||
) -> SandboxSpec {
|
||||
) -> std::result::Result<SandboxSpec, fabro_sandbox::Error> {
|
||||
let clone_origin_url = prepared
|
||||
.git
|
||||
.as_ref()
|
||||
.map(|git| fabro_github::normalize_repo_origin_url(&git.origin_url));
|
||||
let clone_branch = prepared.git.as_ref().map(|git| git.branch.clone());
|
||||
|
||||
match sandbox_provider {
|
||||
SandboxProviderKind::Local => SandboxSpec::Local {
|
||||
working_directory: prepared.source_directory.clone(),
|
||||
},
|
||||
Ok(match sandbox_provider {
|
||||
SandboxProviderKind::Local => {
|
||||
let working_directory = local_working_directory_from_environment(
|
||||
&resolved_run.environment,
|
||||
Some(&prepared.source_directory),
|
||||
)?;
|
||||
SandboxSpec::Local { working_directory }
|
||||
}
|
||||
SandboxProviderKind::Docker => {
|
||||
let mut config = resolve_docker_config(resolved_run);
|
||||
config.skip_clone = true;
|
||||
|
|
@ -884,7 +896,7 @@ fn preflight_sandbox_spec(
|
|||
api_key: daytona_api_key,
|
||||
}
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
async fn run_sandbox_check(
|
||||
|
|
@ -895,13 +907,25 @@ async fn run_sandbox_check(
|
|||
github_app: Option<fabro_github::GitHubCredentials>,
|
||||
daytona_api_key: Option<String>,
|
||||
) -> bool {
|
||||
let spec = preflight_sandbox_spec(
|
||||
let spec = match preflight_sandbox_spec(
|
||||
sandbox_provider,
|
||||
prepared,
|
||||
resolved_run,
|
||||
github_app.clone(),
|
||||
daytona_api_key,
|
||||
);
|
||||
) {
|
||||
Ok(spec) => spec,
|
||||
Err(err) => {
|
||||
checks.push(CheckResult {
|
||||
name: "Sandbox".into(),
|
||||
status: CheckStatus::Error,
|
||||
summary: "failed".into(),
|
||||
details: vec![CheckDetail::new(format!("Provider: {sandbox_provider}"))],
|
||||
remediation: Some(err.to_string()),
|
||||
});
|
||||
return false;
|
||||
}
|
||||
};
|
||||
let sandbox_result: Result<Arc<dyn Sandbox>, String> = spec.build(None).await.map_err(|err| {
|
||||
if matches!(sandbox_provider, SandboxProviderKind::Daytona) {
|
||||
format!("Daytona sandbox creation failed: {err}")
|
||||
|
|
@ -1523,6 +1547,28 @@ enabled = {clone_enabled}
|
|||
(prepared, resolved)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn docker_environment_cwd_is_reported_as_ignored() {
|
||||
let mut resolved = RunNamespace::default();
|
||||
resolved.environment.provider = EnvironmentProvider::Docker;
|
||||
resolved.environment.cwd = Some("/workspace/custom".to_string());
|
||||
|
||||
assert_eq!(environment_capability_warnings(&resolved), vec![
|
||||
"docker provider ignores cwd".to_string()
|
||||
]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn daytona_environment_cwd_is_reported_as_ignored() {
|
||||
let mut resolved = RunNamespace::default();
|
||||
resolved.environment.provider = EnvironmentProvider::Daytona;
|
||||
resolved.environment.cwd = Some("/home/daytona/workspace/custom".to_string());
|
||||
|
||||
assert_eq!(environment_capability_warnings(&resolved), vec![
|
||||
"daytona provider ignores cwd".to_string()
|
||||
]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn prepare_manifest_accepts_project_environment_catalog_definitions() {
|
||||
let mut manifest = minimal_manifest();
|
||||
|
|
@ -1707,12 +1753,12 @@ provider = "local"
|
|||
);
|
||||
|
||||
match spec {
|
||||
SandboxSpec::Docker {
|
||||
Ok(SandboxSpec::Docker {
|
||||
config,
|
||||
clone_origin_url,
|
||||
clone_branch,
|
||||
..
|
||||
} => {
|
||||
}) => {
|
||||
assert!(config.skip_clone);
|
||||
assert_eq!(
|
||||
clone_origin_url.as_deref(),
|
||||
|
|
@ -2116,7 +2162,9 @@ issues = "read"
|
|||
#[tokio::test]
|
||||
async fn preflight_allows_pull_request_enabled_without_github_credentials() {
|
||||
let state = crate::test_support::test_app_state();
|
||||
let source_dir = tempfile::tempdir().unwrap();
|
||||
let mut manifest = minimal_manifest();
|
||||
manifest.cwd = source_dir.path().to_string_lossy().into_owned();
|
||||
manifest.configs.push(types::ManifestConfig {
|
||||
path: Some("/tmp/project/.fabro/project.toml".to_string()),
|
||||
source: Some(
|
||||
|
|
|
|||
|
|
@ -34,6 +34,7 @@ struct EnvironmentListMeta {
|
|||
struct CreateEnvironmentRequest {
|
||||
id: EnvironmentId,
|
||||
provider: EnvironmentProvider,
|
||||
cwd: Option<String>,
|
||||
image: ApiEnvironmentImageSettings,
|
||||
resources: EnvironmentResourcesSettings,
|
||||
network: EnvironmentNetworkSettings,
|
||||
|
|
@ -46,6 +47,7 @@ struct CreateEnvironmentRequest {
|
|||
#[serde(deny_unknown_fields)]
|
||||
struct ReplaceEnvironmentRequest {
|
||||
provider: EnvironmentProvider,
|
||||
cwd: Option<String>,
|
||||
image: ApiEnvironmentImageSettings,
|
||||
resources: EnvironmentResourcesSettings,
|
||||
network: EnvironmentNetworkSettings,
|
||||
|
|
@ -81,6 +83,7 @@ impl CreateEnvironmentRequest {
|
|||
id: self.id,
|
||||
settings: EnvironmentSettings {
|
||||
provider: self.provider,
|
||||
cwd: self.cwd,
|
||||
image: self.image.into_settings()?,
|
||||
resources: self.resources,
|
||||
network: self.network,
|
||||
|
|
@ -96,6 +99,7 @@ impl ReplaceEnvironmentRequest {
|
|||
fn into_settings(self) -> Result<EnvironmentSettings, ApiError> {
|
||||
Ok(EnvironmentSettings {
|
||||
provider: self.provider,
|
||||
cwd: self.cwd,
|
||||
image: self.image.into_settings()?,
|
||||
resources: self.resources,
|
||||
network: self.network,
|
||||
|
|
|
|||
|
|
@ -176,11 +176,14 @@ async fn list_environments_returns_seeded_catalog_sorted_by_id() {
|
|||
#[tokio::test]
|
||||
async fn create_environment_persists_sibling_toml_and_is_visible() {
|
||||
let (app, _temp_dir, environment_dir) = environment_app();
|
||||
let mut body = environment_body("custom-env", "docker");
|
||||
body["cwd"] = json!("/workspace/custom");
|
||||
|
||||
let created = create_environment(&app, "custom-env", "docker").await;
|
||||
let created = create_environment_with_body(&app, &body).await;
|
||||
|
||||
assert_eq!(created["id"], "custom-env");
|
||||
assert_eq!(created["provider"], "docker");
|
||||
assert_eq!(created["cwd"], "/workspace/custom");
|
||||
assert!(environment_dir.join("custom-env.toml").exists());
|
||||
|
||||
let retrieved = app
|
||||
|
|
@ -195,6 +198,7 @@ async fn create_environment_persists_sibling_toml_and_is_visible() {
|
|||
)
|
||||
.await;
|
||||
assert_eq!(retrieved["id"], "custom-env");
|
||||
assert_eq!(retrieved["cwd"], "/workspace/custom");
|
||||
|
||||
let list = app
|
||||
.oneshot(empty_request(Method::GET, "/environments"))
|
||||
|
|
@ -215,6 +219,10 @@ async fn create_environment_persists_sibling_toml_and_is_visible() {
|
|||
persisted.get("provider").and_then(toml::Value::as_str),
|
||||
Some("docker")
|
||||
);
|
||||
assert_eq!(
|
||||
persisted.get("cwd").and_then(toml::Value::as_str),
|
||||
Some("/workspace/custom")
|
||||
);
|
||||
assert!(persisted.get("id").is_none());
|
||||
assert!(persisted.get("revision").is_none());
|
||||
}
|
||||
|
|
@ -254,6 +262,7 @@ async fn replace_environment_updates_file_and_returns_new_etag() {
|
|||
let revision = revision_from(&created);
|
||||
let mut replacement = environment_settings("local");
|
||||
replacement["labels"] = json!({ "tier": "dev" });
|
||||
replacement["cwd"] = json!("/srv/fabro/local");
|
||||
|
||||
let response = app
|
||||
.oneshot(request_with_if_match(
|
||||
|
|
@ -281,6 +290,7 @@ async fn replace_environment_updates_file_and_returns_new_etag() {
|
|||
|
||||
assert_eq!(body["provider"], "local");
|
||||
assert_eq!(body["labels"]["tier"], "dev");
|
||||
assert_eq!(body["cwd"], "/srv/fabro/local");
|
||||
assert_ne!(body["revision"], revision);
|
||||
assert_eq!(etag, format!("\"{}\"", revision_from(&body)));
|
||||
let persisted = persisted_environment_toml(&environment_dir, "replace-env").await;
|
||||
|
|
@ -292,6 +302,10 @@ async fn replace_environment_updates_file_and_returns_new_etag() {
|
|||
.and_then(toml::Value::as_str),
|
||||
Some("dev")
|
||||
);
|
||||
assert_eq!(
|
||||
persisted.get("cwd").and_then(toml::Value::as_str),
|
||||
Some("/srv/fabro/local")
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
|
@ -524,6 +538,31 @@ async fn invalid_environment_settings_return_unprocessable_entity() {
|
|||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn relative_environment_cwd_over_rest_returns_unprocessable_entity() {
|
||||
let (app, _temp_dir, environment_dir) = environment_app();
|
||||
let mut body = environment_body("relative-cwd", "local");
|
||||
body["cwd"] = json!("relative/workspace");
|
||||
|
||||
let response = app
|
||||
.oneshot(json_request(Method::POST, "/environments", &body))
|
||||
.await
|
||||
.expect("invalid cwd create should respond");
|
||||
let error = response_json(
|
||||
response,
|
||||
StatusCode::UNPROCESSABLE_ENTITY,
|
||||
"POST /api/v1/environments relative cwd",
|
||||
)
|
||||
.await;
|
||||
|
||||
assert!(!environment_dir.join("relative-cwd.toml").exists());
|
||||
let message = serde_json::to_string(&error).expect("error should serialize");
|
||||
assert!(
|
||||
message.contains("environment.cwd") && message.contains("absolute path"),
|
||||
"unexpected error: {message}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn dockerfile_path_over_rest_is_rejected_without_persisting_or_exposing_contents() {
|
||||
let (app, temp_dir, environment_dir) = environment_app();
|
||||
|
|
|
|||
|
|
@ -788,6 +788,8 @@ impl Default for EnvironmentLifecycleSettings {
|
|||
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
|
||||
pub struct EnvironmentSettings {
|
||||
pub provider: EnvironmentProvider,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub cwd: Option<String>,
|
||||
pub image: EnvironmentImageSettings,
|
||||
pub resources: EnvironmentResourcesSettings,
|
||||
pub network: EnvironmentNetworkSettings,
|
||||
|
|
@ -800,6 +802,7 @@ impl Default for EnvironmentSettings {
|
|||
fn default() -> Self {
|
||||
Self {
|
||||
provider: EnvironmentProvider::Local,
|
||||
cwd: None,
|
||||
image: EnvironmentImageSettings::default(),
|
||||
resources: EnvironmentResourcesSettings::default(),
|
||||
network: EnvironmentNetworkSettings::default(),
|
||||
|
|
@ -814,6 +817,8 @@ impl Default for EnvironmentSettings {
|
|||
pub struct RunEnvironmentSettings {
|
||||
pub id: String,
|
||||
pub provider: EnvironmentProvider,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub cwd: Option<String>,
|
||||
pub image: EnvironmentImageSettings,
|
||||
pub resources: EnvironmentResourcesSettings,
|
||||
pub network: EnvironmentNetworkSettings,
|
||||
|
|
@ -828,6 +833,7 @@ impl RunEnvironmentSettings {
|
|||
Self {
|
||||
id,
|
||||
provider: environment.provider,
|
||||
cwd: environment.cwd,
|
||||
image: environment.image,
|
||||
resources: environment.resources,
|
||||
network: environment.network,
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
use std::collections::HashMap;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::path::Path;
|
||||
use std::sync::{Arc, Mutex};
|
||||
use std::time::{Duration, Instant};
|
||||
|
||||
|
|
@ -11,6 +11,7 @@ use fabro_model::{Catalog, FallbackTarget, ProviderId};
|
|||
use fabro_sandbox::daytona::DaytonaConfig;
|
||||
use fabro_sandbox::from_environment::{
|
||||
daytona_config_from_environment, docker_config_from_environment,
|
||||
local_working_directory_from_environment,
|
||||
};
|
||||
use fabro_sandbox::{DockerSandboxOptions, SandboxSpec};
|
||||
use fabro_static::EnvVars;
|
||||
|
|
@ -348,10 +349,6 @@ impl RunSession {
|
|||
async fn new(persisted: &Persisted, services: StartServices) -> Result<Self, Error> {
|
||||
let record = persisted.run_spec();
|
||||
let settings = &record.settings;
|
||||
let working_directory = record
|
||||
.source_directory
|
||||
.as_deref()
|
||||
.map_or_else(|| PathBuf::from("."), PathBuf::from);
|
||||
let state = services
|
||||
.run_store
|
||||
.state()
|
||||
|
|
@ -372,7 +369,6 @@ impl RunSession {
|
|||
accepted_definition.map(|definition| Arc::new(definition.workflow_bundle()));
|
||||
|
||||
let resolved = &settings.run;
|
||||
|
||||
let sandbox_provider =
|
||||
resolve_sandbox_provider(resolved).effective_for(resolved.execution.mode);
|
||||
let catalog = Arc::clone(&services.catalog);
|
||||
|
|
@ -387,9 +383,19 @@ impl RunSession {
|
|||
.collect();
|
||||
|
||||
let sandbox = match sandbox_provider {
|
||||
SandboxProviderKind::Local => SandboxSpec::Local {
|
||||
working_directory: working_directory.clone(),
|
||||
},
|
||||
SandboxProviderKind::Local => {
|
||||
let working_directory = local_working_directory_from_environment(
|
||||
&resolved.environment,
|
||||
record.source_directory.as_deref().map(Path::new),
|
||||
)
|
||||
.map_err(|err| {
|
||||
Error::engine_with_source(
|
||||
"Failed to resolve local environment working directory",
|
||||
err,
|
||||
)
|
||||
})?;
|
||||
SandboxSpec::Local { working_directory }
|
||||
}
|
||||
SandboxProviderKind::Docker => SandboxSpec::Docker {
|
||||
config: resolve_docker_config(resolved),
|
||||
github_app: services.github_app.clone(),
|
||||
|
|
@ -1121,6 +1127,7 @@ async fn persist_detached_failure(
|
|||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::collections::HashMap;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::atomic::{AtomicBool, Ordering};
|
||||
use std::sync::{Arc, Mutex};
|
||||
use std::time::Duration;
|
||||
|
|
|
|||
|
|
@ -35,6 +35,10 @@ import type { EnvironmentResourcesSettings } from './environment-resources-setti
|
|||
export interface CreateEnvironmentRequest {
|
||||
'id': string;
|
||||
'provider': EnvironmentProvider;
|
||||
/**
|
||||
* Local-provider command working directory for this environment. Docker and Daytona ignore this value.
|
||||
*/
|
||||
'cwd'?: string | null;
|
||||
'image': EnvironmentApiImageSettings;
|
||||
'resources': EnvironmentResourcesSettings;
|
||||
'network': EnvironmentNetworkSettings;
|
||||
|
|
|
|||
|
|
@ -31,6 +31,10 @@ import type { EnvironmentResourcesSettings } from './environment-resources-setti
|
|||
|
||||
export interface EnvironmentSettings {
|
||||
'provider': EnvironmentProvider;
|
||||
/**
|
||||
* Local-provider command working directory for this environment. Docker and Daytona ignore this value.
|
||||
*/
|
||||
'cwd'?: string | null;
|
||||
'image': EnvironmentImageSettings;
|
||||
'resources': EnvironmentResourcesSettings;
|
||||
'network': EnvironmentNetworkSettings;
|
||||
|
|
|
|||
|
|
@ -39,6 +39,10 @@ export interface Environment {
|
|||
*/
|
||||
'revision': string;
|
||||
'provider': EnvironmentProvider;
|
||||
/**
|
||||
* Local-provider command working directory for this environment. Docker and Daytona ignore this value.
|
||||
*/
|
||||
'cwd'?: string | null;
|
||||
'image': EnvironmentApiImageSettings;
|
||||
'resources': EnvironmentResourcesSettings;
|
||||
'network': EnvironmentNetworkSettings;
|
||||
|
|
|
|||
|
|
@ -34,6 +34,10 @@ import type { EnvironmentResourcesSettings } from './environment-resources-setti
|
|||
*/
|
||||
export interface ReplaceEnvironmentRequest {
|
||||
'provider': EnvironmentProvider;
|
||||
/**
|
||||
* Local-provider command working directory for this environment. Docker and Daytona ignore this value.
|
||||
*/
|
||||
'cwd'?: string | null;
|
||||
'image': EnvironmentApiImageSettings;
|
||||
'resources': EnvironmentResourcesSettings;
|
||||
'network': EnvironmentNetworkSettings;
|
||||
|
|
|
|||
|
|
@ -32,6 +32,10 @@ import type { EnvironmentResourcesSettings } from './environment-resources-setti
|
|||
export interface RunEnvironmentSettings {
|
||||
'id': string;
|
||||
'provider': EnvironmentProvider;
|
||||
/**
|
||||
* Local-provider command working directory for this environment. Docker and Daytona ignore this value.
|
||||
*/
|
||||
'cwd'?: string | null;
|
||||
'image': EnvironmentImageSettings;
|
||||
'resources': EnvironmentResourcesSettings;
|
||||
'network': EnvironmentNetworkSettings;
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue