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

Simplify the CLI run-intent create path

Browse source 
Quality pass over the intent-producer changes, no behavior changes
intended:

- Move the TOML->JSON scalar conversion into fabro-types as
  toml_scalar_to_json_value, next to its inverse, with typed errors and
  round-trip tests; the CLI now calls the shared helper.
- Reuse goal_layer_from_args for --goal/--goal-file resolution instead
  of a second copy of the exclusivity check and cwd anchoring.
- Delete the dead run_manifest_args helper and the test that kept it
  compiling; preflight_manifest_args is the remaining real builder.
- Make run_target_for_environment a pure (provider, cwd) -> target
  mapping using is_clone_based(), warning at the call site, and default
  the environment id from DEFAULT_ENVIRONMENT_ID instead of a literal.
- Resolve the parent run and retrieve the environment concurrently.
- Drop the ResolvedCommandSettings pass-through struct and the
  duplicated parse-error mapping in the project settings presence read.
- Share the environment/workflow-version/git test mocks from the cmd
  test support module instead of three per-file copies.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Scott Werner 2026-09-01 14:01:00 -04:00
parent afe1133878
commit 694d1981ff
12 changed files with 327 additions and 341 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

82
lib/apps/fabro-cli/src/command_context.rs
View file

@ -48,13 +48,6 @@ pub(crate) struct CommandContext {
catalog: OnceLock<Arc<Catalog>>,
}
struct ResolvedCommandSettings {
storage_dir: PathBuf,
run_settings: std::result::Result<RunNamespace, SharedError>,
user_settings: UserSettings,
run_settings_key_presence: RunSettingsKeyPresence,
}
impl CommandContext {
pub(crate) fn from_disk(cli_layer: &CliLayer, process_local_json: bool) -> Result<Self> {
let resolved_settings = load_merged_settings(cli_layer, &ServerMode::None)?;
@ -235,13 +228,10 @@ impl CommandContext {
}
}
fn load_merged_settings(
cli_layer: &CliLayer,
server_mode: &ServerMode,
) -> Result<ResolvedCommandSettings> {
let loaded_settings = match server_mode {
fn load_merged_settings(cli_layer: &CliLayer, server_mode: &ServerMode) -> Result<LoadedSettings> {
match server_mode {
ServerMode::None | ServerMode::ByTarget { .. } => {
user_config::load_resolved_settings(None, None, Some(cli_layer))?
user_config::load_resolved_settings(None, None, Some(cli_layer))
}
ServerMode::ByStorageDir {
storage_dir_override,
@ -250,17 +240,7 @@ fn load_merged_settings(
None,
storage_dir_override.as_deref(),
Some(cli_layer),
)?,
};
Ok(resolve_command_settings(loaded_settings))
}
fn resolve_command_settings(loaded_settings: LoadedSettings) -> ResolvedCommandSettings {
ResolvedCommandSettings {
storage_dir: loaded_settings.storage_dir,
run_settings: loaded_settings.run_settings,
user_settings: loaded_settings.user_settings,
run_settings_key_presence: loaded_settings.run_settings_key_presence,
),
}
}
@ -274,7 +254,7 @@ mod tests {
use fabro_util::printer::Printer;
use tokio::sync::OnceCell;
use super::{CommandContext, ServerMode, resolve_command_settings};
use super::{CommandContext, ServerMode};
use crate::user_config;
fn cli_layer_with_json_and_verbose() -> CliLayer {
@ -289,10 +269,9 @@ mod tests {
fn synthetic_context(process_local_json: bool, printer: Printer) -> CommandContext {
let cli_layer = cli_layer_with_json_and_verbose();
let resolved_settings = resolve_command_settings(
let resolved_settings =
user_config::load_resolved_settings_from_toml("_version = 1\n", None, Some(&cli_layer))
.expect("settings should resolve"),
);
.expect("settings should resolve");
CommandContext {
printer,
process_local_json,
@ -326,32 +305,28 @@ mod tests {
#[test]
fn storage_dir_override_only_changes_storage_root_in_merged_settings() {
let cli_layer = cli_layer_with_json_and_verbose();
let base_settings = resolve_command_settings(
user_config::load_resolved_settings_from_toml(
r#"
let base_settings = user_config::load_resolved_settings_from_toml(
r#"
_version = 1
[server.storage]
root = "/srv/fabro/default"
"#,
None,
Some(&cli_layer),
)
.expect("base settings should resolve"),
);
let connection_settings = resolve_command_settings(
user_config::load_resolved_settings_from_toml(
r#"
None,
Some(&cli_layer),
)
.expect("base settings should resolve");
let connection_settings = user_config::load_resolved_settings_from_toml(
r#"
_version = 1
[server.storage]
root = "/srv/fabro/default"
"#,
Some(std::path::Path::new("/srv/fabro/override")),
Some(&cli_layer),
)
.expect("connection settings should resolve"),
);
Some(std::path::Path::new("/srv/fabro/override")),
Some(&cli_layer),
)
.expect("connection settings should resolve");
assert_eq!(
base_settings.user_settings,
@ -395,27 +370,24 @@ root = "/srv/fabro"
"fixture intentionally omits [server.auth] so server_settings should fail to resolve"
);
let resolved = resolve_command_settings(loaded);
assert_eq!(resolved.storage_dir, PathBuf::from("/srv/fabro"));
assert!(resolved.run_settings.is_ok());
assert_eq!(loaded.storage_dir, PathBuf::from("/srv/fabro"));
assert!(loaded.run_settings.is_ok());
}
#[test]
fn run_settings_include_run_agent_mcps() {
let resolved = resolve_command_settings(
user_config::load_resolved_settings_from_toml(
r#"
let resolved = user_config::load_resolved_settings_from_toml(
r#"
_version = 1
[run.agent.mcps.demo]
type = "stdio"
command = ["demo-mcp"]
"#,
None,
Some(&CliLayer::default()),
)
.expect("settings should resolve"),
);
None,
Some(&CliLayer::default()),
)
.expect("settings should resolve");
let run_settings = resolved.run_settings.expect("run settings should resolve");
assert!(run_settings.agent.mcps.contains_key("demo"));

107
lib/apps/fabro-cli/src/commands/run/create.rs
View file

@ -2,6 +2,7 @@ use std::path::Path;
use anyhow::{Context as _, anyhow, bail};
use fabro_config::project;
use fabro_environment::DEFAULT_ENVIRONMENT_ID;
use fabro_server::manifest_validation;
use fabro_types::settings::run::EnvironmentProvider;
use fabro_types::{DirtyStatus, RunId, RunIntent, RunTarget};
@ -18,10 +19,10 @@ pub(crate) struct CreatedRun {
pub(crate) run_id: RunId,
}
/// Create a workflow run: allocate run directory, persist RunSpec, return
/// (run_id, run_dir).
/// Register the local workflow version closure with the server and create a
/// run from an immutable workflow intent, leaving it in the submitted state.
///
/// This does NOT execute the workflow — it only prepares the run directory.
/// This does NOT start the workflow — starting is a separate request.
pub(crate) async fn create_run(
ctx: &CommandContext,
args: &RunArgs,
@ -43,8 +44,7 @@ pub(crate) async fn create_run(
workflow_path,
&canonical_cwd,
Some(&user_workflows_root),
)
.map_err(anyhow::Error::new)?;
)?;
let prepared = prepare_intent_overrides(args, &canonical_cwd)?;
let validation = manifest_validation::validate_collected_workflow(
package.closure(),
@ -80,17 +80,35 @@ pub(crate) async fn create_run(
}
let client = ctx.server().await?;
let parent_id = match args.parent.as_deref() {
Some(parent_selector) => Some(resolve_run_id(client.as_ref(), parent_selector).await?),
None => None,
};
let environment_id = args.environment.as_deref().unwrap_or("default");
let environment = client
.retrieve_environment(environment_id)
.await
.with_context(|| format!("could not retrieve environment `{environment_id}`"))?;
let target =
run_target_for_environment(environment.settings.provider, &canonical_cwd, ctx, styles)?;
let environment_id = args
.environment
.as_deref()
.unwrap_or(DEFAULT_ENVIRONMENT_ID);
let (parent_id, environment) = tokio::try_join!(
async {
match args.parent.as_deref() {
Some(parent_selector) => Ok(Some(
resolve_run_id(client.as_ref(), parent_selector).await?,
)),
None => Ok(None),
}
},
async {
client
.retrieve_environment(environment_id)
.await
.with_context(|| format!("could not retrieve environment `{environment_id}`"))
},
)?;
let (target, dirty_worktree) =
run_target_for_environment(environment.settings.provider, &canonical_cwd)?;
if dirty_worktree {
fabro_util::printerr!(
ctx.printer(),
"{} the caller Git working tree is dirty; uncommitted changes are not included in the run target.",
styles.yellow.apply_to("Warning:"),
);
}
let workflow_version_id = package.closure().root_id();
client
.register_workflow_versions(
@ -138,42 +156,33 @@ fn warn_untransmitted_settings(
);
}
/// Derives the run target from the caller directory for the environment's
/// provider. Returns the target plus whether a clone-based observation found a
/// dirty Git worktree, so the caller can warn about it.
fn run_target_for_environment(
provider: EnvironmentProvider,
canonical_cwd: &Path,
ctx: &CommandContext,
styles: &Styles,
) -> anyhow::Result<RunTarget> {
match provider {
EnvironmentProvider::Local => {
let path = canonical_cwd.to_str().ok_or_else(|| {
anyhow!(
"caller working directory is not valid UTF-8: {}",
canonical_cwd.display()
)
})?;
Ok(RunTarget::Folder {
) -> anyhow::Result<(RunTarget, bool)> {
if !provider.is_clone_based() {
let path = canonical_cwd.to_str().ok_or_else(|| {
anyhow!(
"caller working directory is not valid UTF-8: {}",
canonical_cwd.display()
)
})?;
return Ok((
RunTarget::Folder {
path: path.to_string(),
})
}
EnvironmentProvider::Docker | EnvironmentProvider::Daytona => {
let Some(observation) = fabro_manifest::observe_git_run_target(canonical_cwd, None)
else {
return Ok(RunTarget::None {});
};
if observation.legacy_git_context.dirty == DirtyStatus::Dirty {
fabro_util::printerr!(
ctx.printer(),
"{} the caller Git working tree is dirty; uncommitted changes are not included in the run target.",
styles.yellow.apply_to("Warning:"),
);
}
let target = observation.run_target.ok_or_else(|| {
anyhow!(
"the caller Git checkout cannot be represented as a canonical GitHub run target"
)
})?;
Ok(RunTarget::Git(target))
}
},
false,
));
}
let Some(observation) = fabro_manifest::observe_git_run_target(canonical_cwd, None) else {
return Ok((RunTarget::None {}, false));
};
let dirty = observation.legacy_git_context.dirty == DirtyStatus::Dirty;
let target = observation.run_target.ok_or_else(|| {
anyhow!("the caller Git checkout cannot be represented as a canonical GitHub run target")
})?;
Ok((RunTarget::Git(target), dirty))
}

36
lib/apps/fabro-cli/src/commands/run/overrides.rs
View file

@ -1,7 +1,7 @@
use std::collections::HashMap;
use std::path::{Path, PathBuf};
use anyhow::{Context as _, Result, anyhow, bail};
use anyhow::{Context as _, Result, anyhow};
use fabro_config::{
CliLayer, CliOutputLayer, RunGoalLayer, RunLayer, parse_input_overrides, parse_labels,
};
@ -84,39 +84,23 @@ pub(super) fn prepare_intent_overrides(
args: &RunArgs,
cwd: &Path,
) -> Result<PreparedIntentOverrides> {
let goal = match (args.goal.as_deref(), args.goal_file.as_deref()) {
(Some(_), Some(_)) => {
bail!("--goal and --goal-file are mutually exclusive; use exactly one")
}
(Some(goal), None) => Some(goal.to_string()),
(None, Some(path)) => {
let absolute = if path.is_absolute() {
path.to_path_buf()
} else {
cwd.join(path)
};
let goal = match goal_layer_from_args(args.goal.as_deref(), args.goal_file.as_deref(), cwd)? {
None => None,
Some(RunGoalLayer::Inline(goal)) => Some(goal.as_source()),
Some(RunGoalLayer::File { file }) => {
let path = PathBuf::from(file.as_source());
Some(
std::fs::read_to_string(&absolute)
.with_context(|| format!("failed to read goal file {}", absolute.display()))?,
std::fs::read_to_string(&path)
.with_context(|| format!("failed to read goal file {}", path.display()))?,
)
}
(None, None) => None,
};
let input_overrides = parse_input_overrides(&args.inputs.values)?;
let inputs = input_overrides
.iter()
.map(|(key, value)| {
let value = match value {
toml::Value::String(value) => serde_json::Value::String(value.clone()),
toml::Value::Integer(value) => serde_json::Value::Number((*value).into()),
toml::Value::Float(value) => serde_json::Number::from_f64(*value)
.map(serde_json::Value::Number)
.ok_or_else(|| anyhow!("input override `{key}` must be a finite float"))?,
toml::Value::Boolean(value) => serde_json::Value::Bool(*value),
toml::Value::Datetime(_) | toml::Value::Array(_) | toml::Value::Table(_) => {
bail!("input override `{key}` must be a scalar value")
}
};
let value = fabro_types::toml_scalar_to_json_value(value)
.map_err(|error| anyhow!("input override `{key}` {error}"))?;
Ok((key.clone(), value))
})
.collect::<Result<HashMap<_, _>>>()?;

14
lib/apps/fabro-cli/src/main.rs
View file

@ -1312,20 +1312,6 @@ destination = "{destination}"
}
}
#[test]
fn run_manifest_args_preserves_input_only_manifest_args() {
let cli = Cli::try_parse_from(["fabro", "run", "workflow.toml", "-I", "foo=bar"])
.expect("should parse");
match *cli.command.unwrap() {
Commands::RunCmd(RunCommands::Run(args)) => {
let manifest_args = manifest_args::run_manifest_args(&args)
.expect("input-only args should be retained");
assert_eq!(manifest_args.input, vec!["foo=bar"]);
}
_ => panic!("unexpected command variant"),
}
}
#[test]
fn parse_create_input_long_flag() {
let cli = Cli::try_parse_from(["fabro", "create", "workflow.toml", "--input", "foo=bar"])

18
lib/apps/fabro-cli/src/manifest_args.rs
View file

@ -1,24 +1,6 @@
use fabro_api::types;
use crate::args::PreflightArgs;
#[cfg(test)]
use crate::args::RunArgs;
#[cfg(test)]
pub(crate) fn run_manifest_args(args: &RunArgs) -> Option<types::ManifestArgs> {
let payload = types::ManifestArgs {
auto_approve: args.auto_approve.then_some(true),
dry_run: args.dry_run.then_some(true),
label: args.label.clone(),
model: args.model.clone(),
preserve_sandbox: args.preserve_sandbox.then_some(true),
provider: args.provider.clone(),
environment: args.environment.clone(),
input: args.inputs.values.clone(),
verbose: args.verbose.then_some(true),
};
(!fabro_manifest::manifest_args_is_empty(&payload)).then_some(payload)
}
pub(crate) fn preflight_manifest_args(args: &PreflightArgs) -> Option<types::ManifestArgs> {
let payload = types::ManifestArgs {

19
lib/apps/fabro-cli/src/user_config.rs
View file

@ -91,21 +91,14 @@ pub(crate) fn load_resolved_settings(
pub(crate) fn read_project_run_settings_key_presence(
path: &Path,
) -> anyhow::Result<RunSettingsKeyPresence> {
let parse_error =
|source| fabro_config::Error::parse_file("Failed to parse settings file", path, source);
let source = std::fs::read_to_string(path)
.map_err(|source| fabro_config::Error::read_file(path, source))?;
let document: toml::Value = toml::from_str(&source).map_err(|source| {
fabro_config::Error::parse_file(
"Failed to parse settings file",
path,
ParseError::Toml(source.to_string()),
)
})?;
let layer = source.parse::<SettingsLayer>().map_err(|source| {
fabro_config::Error::parse_file("Failed to parse settings file", path, source)
})?;
validate_settings_source(&layer, SettingsSource::Project).map_err(|source| {
fabro_config::Error::parse_file("Failed to parse settings file", path, source)
})?;
let document: toml::Value = toml::from_str(&source)
.map_err(|source| parse_error(ParseError::Toml(source.to_string())))?;
let layer = source.parse::<SettingsLayer>().map_err(parse_error)?;
validate_settings_source(&layer, SettingsSource::Project).map_err(parse_error)?;
Ok(RunSettingsKeyPresence::from_document(&document))
}

131
lib/apps/fabro-cli/tests/it/cmd/create.rs
View file

@ -12,8 +12,10 @@ use insta::assert_snapshot;
use serde_json::json;
use super::support::{
created_run_id, fixture, output_stderr, output_stdout, remote_run_summary_json, resolve_run,
run_count_for_test_case, run_state,
created_run_id, environment_json, fixture, mock_environment,
mock_workflow_version_registrations, mock_workflow_version_registrations_recording,
output_stderr, output_stdout, remote_run_summary_json, resolve_run, run_count_for_test_case,
run_git, run_state,
};
use crate::support::unique_run_id;
@ -36,56 +38,6 @@ fn run_status_response(run_id: &str, status: &str) -> serde_json::Value {
)
}
fn environment_json(id: &str, provider: &str) -> serde_json::Value {
json!({
"id": id,
"revision": "0".repeat(64),
"provider": provider,
"image": { "docker": null, "dockerfile": null },
"resources": { "cpu": null, "memory": null, "disk": null },
"network": { "mode": "allow_all", "allow": [] },
"lifecycle": {
"preserve": false,
"stop_on_terminal": true,
"auto_stop": null
},
"labels": {},
"env": {}
})
}
fn mock_environment<'a>(server: &'a MockServer, id: &str, provider: &str) -> Mock<'a> {
server.mock(|when, then| {
when.method("GET")
.path(format!("/api/v1/environments/{id}"));
then.status(200)
.header("content-type", "application/json")
.json_body(environment_json(id, provider));
})
}
fn mock_workflow_version_registrations(server: &MockServer) -> Mock<'_> {
server.mock(|when, then| {
when.method("POST").path("/api/v1/workflow-versions");
then.respond_with(|request| {
let version: fabro_types::WorkflowVersion = serde_json::from_slice(request.body_ref())
.expect("workflow-version request body should be valid JSON");
HttpMockResponse::builder()
.status(201)
.header("content-type", "application/json")
.body(
json!({
"workflow_version_id": version
.id()
.expect("mocked workflow version should have a valid ID")
})
.to_string(),
)
.build()
});
})
}
fn mock_intent_create<'a>(
server: &'a MockServer,
run_id: &str,
@ -126,31 +78,6 @@ fn write_workflow(root: &std::path::Path, directory: &str, graph_name: &str) ->
directory.join("workflow.toml")
}
#[expect(
clippy::disallowed_methods,
reason = "the integration fixture needs a real local Git checkout"
)]
fn run_git(path: &std::path::Path, args: &[&str]) -> String {
let output = std::process::Command::new("git")
.args(args)
.current_dir(path)
.output()
.expect("Git fixture command should execute");
assert!(
output.status.success(),
"git {args:?} failed: {}",
String::from_utf8_lossy(&output.stderr)
);
String::from_utf8(output.stdout)
.expect("Git fixture output should be UTF-8")
.trim()
.to_string()
}
fn init_git_repository(path: &std::path::Path) {
run_git(path, &["init", "--quiet"]);
}
#[test]
fn help() {
let context = test_context!();
@ -235,23 +162,8 @@ fn create_defers_provider_validation_to_the_server() {
let run_id = unique_run_id();
let environment_mock = mock_environment(&server, "default", "docker");
let registered_versions = Arc::new(Mutex::new(Vec::new()));
let registered_versions_for_mock = Arc::clone(&registered_versions);
let version_mock = server.mock(|when, then| {
when.method("POST").path("/api/v1/workflow-versions");
then.respond_with(move |request| {
let version: fabro_types::WorkflowVersion =
serde_json::from_slice(request.body_ref()).unwrap();
registered_versions_for_mock
.lock()
.unwrap()
.push(serde_json::from_slice::<serde_json::Value>(request.body_ref()).unwrap());
HttpMockResponse::builder()
.status(201)
.header("content-type", "application/json")
.body(json!({ "workflow_version_id": version.id().unwrap() }).to_string())
.build()
});
});
let version_mock =
mock_workflow_version_registrations_recording(&server, Arc::clone(&registered_versions));
let mock = server.mock(|when, then| {
when.method("POST").path("/api/v1/runs");
then.status(201)
@ -617,7 +529,7 @@ fn create_preserves_named_user_other_checkout_and_loose_file_selection() {
let project = tempfile::tempdir().unwrap();
let outside = tempfile::tempdir().unwrap();
let other_checkout = tempfile::tempdir().unwrap();
init_git_repository(project.path());
run_git(project.path(), &["init", "--quiet"]);
let project_workflow = write_workflow(project.path(), ".fabro/workflows/hello", "ProjectHello");
let user_root = context.home_dir.join(".fabro/workflows");
let user_workflow = write_workflow(&user_root, "hello", "UserHello");
@ -969,23 +881,8 @@ fn create_registers_dependencies_before_the_root_and_then_creates_once() {
let run_id = unique_run_id();
let environment_mock = mock_environment(&server, "local", "local");
let registrations = Arc::new(Mutex::new(Vec::new()));
let registrations_for_mock = Arc::clone(&registrations);
let version_mock = server.mock(|when, then| {
when.method("POST").path("/api/v1/workflow-versions");
then.respond_with(move |request| {
let version: fabro_types::WorkflowVersion =
serde_json::from_slice(request.body_ref()).unwrap();
registrations_for_mock
.lock()
.unwrap()
.push(version.entrypoint().as_str().to_string());
HttpMockResponse::builder()
.status(201)
.header("content-type", "application/json")
.body(json!({ "workflow_version_id": version.id().unwrap() }).to_string())
.build()
});
});
let version_mock =
mock_workflow_version_registrations_recording(&server, Arc::clone(&registrations));
let requests = Arc::new(Mutex::new(Vec::<serde_json::Value>::new()));
let requests_for_mock = Arc::clone(&requests);
let registrations_for_create = Arc::clone(&registrations);
@ -1010,7 +907,7 @@ fn create_registers_dependencies_before_the_root_and_then_creates_once() {
});
});
let project = tempfile::tempdir().unwrap();
init_git_repository(project.path());
run_git(project.path(), &["init", "--quiet"]);
write_workflow(project.path(), ".fabro/workflows/root", "Root");
write_workflow(project.path(), ".fabro/workflows/child", "Child");
std::fs::write(
@ -1047,7 +944,13 @@ fn create_registers_dependencies_before_the_root_and_then_creates_once() {
environment_mock.assert();
version_mock.assert_calls(2);
create_mock.assert_calls(1);
assert_eq!(registrations.lock().unwrap().as_slice(), [
let registered_entrypoints: Vec<String> = registrations
.lock()
.unwrap()
.iter()
.map(|version| version["entrypoint"].as_str().unwrap().to_string())
.collect();
assert_eq!(registered_entrypoints, [
".fabro/workflows/child/workflow.fabro",
".fabro/workflows/root/workflow.fabro",
]);

2
lib/apps/fabro-cli/tests/it/cmd/mcp.rs
View file

@ -25,7 +25,7 @@ use fabro_types::{Graph, RunId, WorkflowSettings, test_support};
use httpmock::Method::{GET, POST};
use httpmock::MockServer;
use super::support::{mock_resolved_run, remote_run_summary_json};
use super::support::{mock_resolved_run, remote_run_summary_json, run_git};
use crate::support::{
RealAuthHarness, TEST_DEV_TOKEN, run_projection_json, seed_dev_token_auth, unique_run_id,
};

51
lib/apps/fabro-cli/tests/it/cmd/run.rs
View file

@ -6,11 +6,12 @@
use fabro_config::Storage;
use fabro_test::{fabro_json_snapshot, fabro_snapshot, test_context};
use fabro_vault::{SecretType, Vault};
use httpmock::{HttpMockResponse, Mock, MockServer};
use httpmock::MockServer;
use serde_json::Value;
use super::support::{
created_run_id, output_stderr, remote_run_summary_json, run_state, wait_for_event_names,
created_run_id, mock_environment, mock_workflow_version_registrations, output_stderr,
remote_run_summary_json, run_state, wait_for_event_names,
};
use crate::support::{LightweightCli, run_output_filters, run_projection_json, unique_run_id};
@ -31,52 +32,6 @@ fn run_status_response(run_id: &str, status: &str) -> serde_json::Value {
)
}
fn mock_environment<'a>(server: &'a MockServer, id: &str, provider: &str) -> Mock<'a> {
server.mock(|when, then| {
when.method("GET")
.path(format!("/api/v1/environments/{id}"));
then.status(200)
.header("content-type", "application/json")
.json_body(serde_json::json!({
"id": id,
"revision": "0".repeat(64),
"provider": provider,
"image": { "docker": null, "dockerfile": null },
"resources": { "cpu": null, "memory": null, "disk": null },
"network": { "mode": "allow_all", "allow": [] },
"lifecycle": {
"preserve": false,
"stop_on_terminal": true,
"auto_stop": null
},
"labels": {},
"env": {}
}));
})
}
fn mock_workflow_version_registrations(server: &MockServer) -> Mock<'_> {
server.mock(|when, then| {
when.method("POST").path("/api/v1/workflow-versions");
then.respond_with(|request| {
let version: fabro_types::WorkflowVersion = serde_json::from_slice(request.body_ref())
.expect("workflow-version request body should be valid JSON");
HttpMockResponse::builder()
.status(201)
.header("content-type", "application/json")
.body(
serde_json::json!({
"workflow_version_id": version
.id()
.expect("mocked workflow version should have a valid ID")
})
.to_string(),
)
.build()
});
})
}
fn remote_run_state_response(run_id: &str) -> serde_json::Value {
let mut state = run_projection_json(
run_id,

87
lib/apps/fabro-cli/tests/it/cmd/support.rs
View file

@ -12,6 +12,7 @@
use std::path::{Path, PathBuf};
use std::process::Output;
use std::sync::atomic::{AtomicU64, Ordering};
use std::sync::{Arc, Mutex};
use std::time::{Duration, Instant};
use base64::Engine as _;
@ -23,7 +24,7 @@ use fabro_store::EventEnvelope;
use fabro_test::{TestContext, expect_reqwest_status};
use fabro_types::test_support::test_principal;
use fabro_types::{RunId, StageId};
use httpmock::{Mock, MockServer};
use httpmock::{HttpMockResponse, Mock, MockServer};
use serde_json::Value;
use shlex::try_quote;
@ -141,6 +142,90 @@ pub(crate) fn mock_resolved_run<'a>(
})
}
/// Canonical environment response body for mock servers, matching the
/// `GET /api/v1/environments/{id}` shape the run-intent create path reads.
pub(crate) fn environment_json(id: &str, provider: &str) -> Value {
serde_json::json!({
"id": id,
"revision": "0".repeat(64),
"provider": provider,
"image": { "docker": null, "dockerfile": null },
"resources": { "cpu": null, "memory": null, "disk": null },
"network": { "mode": "allow_all", "allow": [] },
"lifecycle": {
"preserve": false,
"stop_on_terminal": true,
"auto_stop": null
},
"labels": {},
"env": {}
})
}
pub(crate) fn mock_environment<'a>(server: &'a MockServer, id: &str, provider: &str) -> Mock<'a> {
server.mock(|when, then| {
when.method("GET")
.path(format!("/api/v1/environments/{id}"));
then.status(200)
.header("content-type", "application/json")
.json_body(environment_json(id, provider));
})
}
pub(crate) fn mock_workflow_version_registrations(server: &MockServer) -> Mock<'_> {
mock_workflow_version_registrations_recording(server, Arc::new(Mutex::new(Vec::new())))
}
/// Accepts `POST /api/v1/workflow-versions`, echoing each version's
/// content-derived ID back, and records every request body into
/// `registrations` for later assertions.
pub(crate) fn mock_workflow_version_registrations_recording(
server: &MockServer,
registrations: Arc<Mutex<Vec<Value>>>,
) -> Mock<'_> {
server.mock(|when, then| {
when.method("POST").path("/api/v1/workflow-versions");
then.respond_with(move |request| {
let body: Value = serde_json::from_slice(request.body_ref())
.expect("workflow-version request body should be valid JSON");
let version: fabro_types::WorkflowVersion = serde_json::from_value(body.clone())
.expect("workflow-version request body should be a workflow version");
registrations.lock().unwrap().push(body);
HttpMockResponse::builder()
.status(201)
.header("content-type", "application/json")
.body(
serde_json::json!({
"workflow_version_id": version
.id()
.expect("mocked workflow version should have a valid ID")
})
.to_string(),
)
.build()
});
})
}
/// Runs a `git` command in `path` for fixture setup, panicking on failure and
/// returning trimmed stdout.
pub(crate) fn run_git(path: &Path, args: &[&str]) -> String {
let output = std::process::Command::new("git")
.args(args)
.current_dir(path)
.output()
.expect("Git fixture command should execute");
assert!(
output.status.success(),
"git {args:?} failed: {}",
String::from_utf8_lossy(&output.stderr)
);
String::from_utf8(output.stdout)
.expect("Git fixture output should be UTF-8")
.trim()
.to_string()
}
/// Snapshot filter that scrubs short (12-char) ULID suffixes from output, used
/// when the CLI prints abbreviated run IDs.
pub(crate) fn ulid_filter() -> (String, String) {

116
lib/foundation/fabro-types/src/input_scalar.rs
View file

@ -17,6 +17,46 @@ pub enum JsonScalarToTomlError {
NumberOutOfRange,
}
/// The reason a parsed TOML value cannot be represented as a JSON scalar.
#[derive(Debug, Clone, Copy, PartialEq, Eq, thiserror::Error)]
pub enum TomlScalarToJsonError {
/// The TOML float is not finite, and JSON numbers must be finite.
#[error("must be a finite float")]
NonFiniteFloat,
/// TOML datetimes are outside the scalar-only conversion contract.
#[error("must be a scalar value")]
Datetime,
/// TOML arrays are outside the scalar-only conversion contract.
#[error("must be a scalar value")]
Array,
/// TOML tables are outside the scalar-only conversion contract.
#[error("must be a scalar value")]
Table,
}
/// Converts an already-parsed TOML scalar into a JSON value.
///
/// The inverse of [`json_scalar_to_toml_value`]: strings, booleans, and
/// integers map directly, and finite floats become JSON numbers.
///
/// # Errors
///
/// Returns [`TomlScalarToJsonError`] for TOML datetimes, arrays, tables, or a
/// non-finite float (JSON numbers must be finite).
pub fn toml_scalar_to_json_value(value: &toml::Value) -> Result<Value, TomlScalarToJsonError> {
match value {
toml::Value::String(value) => Ok(Value::String(value.clone())),
toml::Value::Integer(value) => Ok(Value::Number((*value).into())),
toml::Value::Float(value) => serde_json::Number::from_f64(*value)
.map(Value::Number)
.ok_or(TomlScalarToJsonError::NonFiniteFloat),
toml::Value::Boolean(value) => Ok(Value::Bool(*value)),
toml::Value::Datetime(_) => Err(TomlScalarToJsonError::Datetime),
toml::Value::Array(_) => Err(TomlScalarToJsonError::Array),
toml::Value::Table(_) => Err(TomlScalarToJsonError::Table),
}
}
/// Converts an already-parsed JSON scalar into a TOML value.
///
/// Numbers are converted to a signed integer first and then to a float. As a
@ -50,7 +90,81 @@ pub fn json_scalar_to_toml_value(value: &Value) -> Result<toml::Value, JsonScala
mod tests {
use serde_json::{Value, json};
use super::{JsonScalarToTomlError, json_scalar_to_toml_value};
use super::{
JsonScalarToTomlError, TomlScalarToJsonError, json_scalar_to_toml_value,
toml_scalar_to_json_value,
};
#[test]
fn converts_toml_scalars_to_json_values() -> Result<(), TomlScalarToJsonError> {
let cases = [
(toml::Value::String("hello".to_string()), json!("hello")),
(toml::Value::Integer(42), json!(42)),
(toml::Value::Float(1.25), json!(1.25)),
(toml::Value::Boolean(true), json!(true)),
];
for (input, expected) in cases {
assert_eq!(toml_scalar_to_json_value(&input)?, expected);
}
Ok(())
}
#[test]
fn rejects_non_scalar_toml_values_with_typed_errors() {
let datetime: toml::Value = "value = 1979-05-27T07:32:00Z"
.parse::<toml::Table>()
.unwrap()
.remove("value")
.unwrap();
let cases = [
(datetime, TomlScalarToJsonError::Datetime),
(
toml::Value::Array(vec![toml::Value::Integer(1)]),
TomlScalarToJsonError::Array,
),
(
toml::Value::Table(toml::Table::new()),
TomlScalarToJsonError::Table,
),
];
for (input, expected) in cases {
assert_eq!(toml_scalar_to_json_value(&input), Err(expected));
}
}
#[test]
fn rejects_non_finite_toml_floats() {
for input in [f64::NAN, f64::INFINITY, f64::NEG_INFINITY] {
assert_eq!(
toml_scalar_to_json_value(&toml::Value::Float(input)),
Err(TomlScalarToJsonError::NonFiniteFloat)
);
}
}
#[test]
fn toml_scalars_round_trip_through_json() -> Result<(), TomlScalarToJsonError> {
let scalars = [
toml::Value::String("hello".to_string()),
toml::Value::Integer(i64::MIN),
toml::Value::Integer(i64::MAX),
toml::Value::Float(1.25),
toml::Value::Boolean(false),
];
for input in scalars {
let json = toml_scalar_to_json_value(&input)?;
assert_eq!(
json_scalar_to_toml_value(&json).expect("round trip should stay scalar"),
input
);
}
Ok(())
}
#[test]
fn converts_strings_to_toml_strings() -> Result<(), JsonScalarToTomlError> {

5
lib/foundation/fabro-types/src/lib.rs
View file

@ -81,7 +81,10 @@ pub use graph::{
AttrValue, AttributeScope, ContextKeyAttr, Edge, Graph, KNOWN_HANDLER_TYPES, Node, OnFailure,
ResolvedOnFailure, is_known_handler_type, is_llm_handler_type, shape_to_handler_type,
};
pub use input_scalar::{JsonScalarToTomlError, json_scalar_to_toml_value};
pub use input_scalar::{
JsonScalarToTomlError, TomlScalarToJsonError, json_scalar_to_toml_value,
toml_scalar_to_json_value,
};
pub use interview::{
InterviewQuestionRecord, QuestionType, ReviewTarget, ReviewTargetError, ReviewTargetKind,
};