mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-08 03:10:26 +00:00
Merge pull request #711 from fabro-sh/remove-dead-docker-image-arg
Remove obsolete per-run Docker image argument
This commit is contained in:
commit
baca460177
8 changed files with 59 additions and 36 deletions
|
|
@ -9184,9 +9184,6 @@ components:
|
|||
environment:
|
||||
type: string
|
||||
description: Named environment slug to select for the run.
|
||||
docker_image:
|
||||
type: string
|
||||
description: Per-run environment image override.
|
||||
verbose:
|
||||
type: boolean
|
||||
dry_run:
|
||||
|
|
|
|||
|
|
@ -75,7 +75,6 @@ pub(crate) fn run_args_overrides(args: &RunArgs) -> Result<ManifestSettingsOverr
|
|||
model: args.model.as_deref(),
|
||||
provider: args.provider.as_deref(),
|
||||
environment: args.environment.as_deref(),
|
||||
docker_image: None,
|
||||
preserve_sandbox: sparse_flag(args.preserve_sandbox),
|
||||
dry_run: sparse_flag(args.dry_run),
|
||||
auto_approve: sparse_flag(args.auto_approve),
|
||||
|
|
@ -98,7 +97,6 @@ pub(crate) fn preflight_args_overrides(args: &PreflightArgs) -> Result<ManifestS
|
|||
model: args.model.as_deref(),
|
||||
provider: args.provider.as_deref(),
|
||||
environment: args.environment.as_deref(),
|
||||
docker_image: None,
|
||||
preserve_sandbox: None,
|
||||
dry_run: None,
|
||||
auto_approve: None,
|
||||
|
|
|
|||
|
|
@ -11,7 +11,6 @@ pub(crate) fn run_manifest_args(args: &RunArgs) -> Option<types::ManifestArgs> {
|
|||
preserve_sandbox: args.preserve_sandbox.then_some(true),
|
||||
provider: args.provider.clone(),
|
||||
environment: args.environment.clone(),
|
||||
docker_image: None,
|
||||
input: args.inputs.values.clone(),
|
||||
verbose: args.verbose.then_some(true),
|
||||
};
|
||||
|
|
@ -27,7 +26,6 @@ pub(crate) fn preflight_manifest_args(args: &PreflightArgs) -> Option<types::Man
|
|||
preserve_sandbox: None,
|
||||
provider: args.provider.clone(),
|
||||
environment: args.environment.clone(),
|
||||
docker_image: None,
|
||||
input: args.inputs.values.clone(),
|
||||
verbose: args.verbose.then_some(true),
|
||||
};
|
||||
|
|
|
|||
|
|
@ -361,7 +361,6 @@ pub(crate) fn manifest_args_overrides(
|
|||
model: args.model.as_deref(),
|
||||
provider: args.provider.as_deref(),
|
||||
environment: args.environment.as_deref(),
|
||||
docker_image: args.docker_image.as_deref(),
|
||||
preserve_sandbox: args.preserve_sandbox,
|
||||
dry_run: args.dry_run,
|
||||
auto_approve: args.auto_approve,
|
||||
|
|
@ -2068,7 +2067,6 @@ root = "/srv/fabro"
|
|||
preserve_sandbox: None,
|
||||
provider: None,
|
||||
environment: None,
|
||||
docker_image: None,
|
||||
input: Vec::new(),
|
||||
verbose: None,
|
||||
});
|
||||
|
|
@ -2101,7 +2099,6 @@ override = "server"
|
|||
preserve_sandbox: None,
|
||||
provider: None,
|
||||
environment: None,
|
||||
docker_image: None,
|
||||
input: vec!["override=cli".to_string()],
|
||||
verbose: None,
|
||||
});
|
||||
|
|
|
|||
|
|
@ -58,7 +58,6 @@ pub fn run_tool_manifest_args(spec: &ValidatedCreateRunSpec) -> Option<types::Ma
|
|||
|
||||
let payload = types::ManifestArgs {
|
||||
auto_approve: spec.auto_approve.filter(|value| *value),
|
||||
docker_image: None,
|
||||
dry_run: spec.dry_run.filter(|value| *value),
|
||||
input,
|
||||
label,
|
||||
|
|
@ -77,7 +76,6 @@ pub fn run_tool_run_overrides(spec: &ValidatedCreateRunSpec) -> Option<RunLayer>
|
|||
model: spec.model.as_deref(),
|
||||
provider: spec.provider.as_deref(),
|
||||
environment: spec.environment.as_deref(),
|
||||
docker_image: None,
|
||||
preserve_sandbox: spec.preserve_sandbox,
|
||||
dry_run: spec.dry_run,
|
||||
auto_approve: spec.auto_approve,
|
||||
|
|
|
|||
|
|
@ -61,7 +61,6 @@ pub struct RunOverrideInput<'a> {
|
|||
pub model: Option<&'a str>,
|
||||
pub provider: Option<&'a str>,
|
||||
pub environment: Option<&'a str>,
|
||||
pub docker_image: Option<&'a str>,
|
||||
pub preserve_sandbox: Option<bool>,
|
||||
pub dry_run: Option<bool>,
|
||||
pub auto_approve: Option<bool>,
|
||||
|
|
@ -79,23 +78,19 @@ pub fn build_run_overrides(input: RunOverrideInput<'_>) -> RunLayer {
|
|||
fallbacks: MergeMap::default(),
|
||||
controls: None,
|
||||
});
|
||||
let environment = (input.environment.is_some()
|
||||
|| input.docker_image.is_some()
|
||||
|| input.preserve_sandbox.is_some())
|
||||
.then(|| RunEnvironmentLayer {
|
||||
id: input.environment.map(ToOwned::to_owned),
|
||||
image: input.docker_image.map(|image| EnvironmentImageLayer {
|
||||
docker: Some(image.to_string()),
|
||||
..EnvironmentImageLayer::default()
|
||||
}),
|
||||
lifecycle: input
|
||||
.preserve_sandbox
|
||||
.map(|preserve| EnvironmentLifecycleLayer {
|
||||
preserve: Some(preserve),
|
||||
..EnvironmentLifecycleLayer::default()
|
||||
}),
|
||||
..RunEnvironmentLayer::default()
|
||||
});
|
||||
let environment =
|
||||
(input.environment.is_some() || input.preserve_sandbox.is_some()).then(|| {
|
||||
RunEnvironmentLayer {
|
||||
id: input.environment.map(ToOwned::to_owned),
|
||||
lifecycle: input
|
||||
.preserve_sandbox
|
||||
.map(|preserve| EnvironmentLifecycleLayer {
|
||||
preserve: Some(preserve),
|
||||
..EnvironmentLifecycleLayer::default()
|
||||
}),
|
||||
..RunEnvironmentLayer::default()
|
||||
}
|
||||
});
|
||||
let execution =
|
||||
(input.dry_run.is_some() || input.auto_approve.is_some()).then(|| RunExecutionLayer {
|
||||
mode: input.dry_run.map(|dry_run| {
|
||||
|
|
@ -894,7 +889,6 @@ pub fn manifest_args_is_empty(args: &types::ManifestArgs) -> bool {
|
|||
&& args.preserve_sandbox.is_none()
|
||||
&& args.provider.is_none()
|
||||
&& args.environment.is_none()
|
||||
&& args.docker_image.is_none()
|
||||
&& args.input.is_empty()
|
||||
&& args.verbose.is_none()
|
||||
}
|
||||
|
|
@ -966,7 +960,6 @@ mod tests {
|
|||
model: Some("gpt-5.4-mini"),
|
||||
provider: Some("openai"),
|
||||
environment: Some("local"),
|
||||
docker_image: None,
|
||||
preserve_sandbox: Some(true),
|
||||
dry_run: Some(true),
|
||||
auto_approve: Some(false),
|
||||
|
|
@ -1028,6 +1021,40 @@ mod tests {
|
|||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sparse_run_overrides_preserve_only_has_no_image() {
|
||||
let overrides = build_sparse_run_overrides(RunOverrideInput {
|
||||
preserve_sandbox: Some(true),
|
||||
..RunOverrideInput::default()
|
||||
})
|
||||
.expect("preserve override");
|
||||
let environment = overrides.environment.expect("environment override");
|
||||
|
||||
assert!(environment.image.is_none());
|
||||
assert_eq!(
|
||||
environment.lifecycle.expect("lifecycle override").preserve,
|
||||
Some(true)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sparse_run_overrides_environment_only_has_no_image() {
|
||||
let overrides = build_sparse_run_overrides(RunOverrideInput {
|
||||
environment: Some("local"),
|
||||
..RunOverrideInput::default()
|
||||
})
|
||||
.expect("environment override");
|
||||
let environment = overrides.environment.expect("environment override");
|
||||
|
||||
assert_eq!(environment.id.as_deref(), Some("local"));
|
||||
assert!(environment.image.is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sparse_run_overrides_default_is_empty() {
|
||||
assert!(build_sparse_run_overrides(RunOverrideInput::default()).is_none());
|
||||
}
|
||||
|
||||
// Regression coverage for https://github.com/fabro-sh/fabro/issues/476.
|
||||
#[test]
|
||||
fn build_manifest_bundles_agent_output_schema_file() {
|
||||
|
|
|
|||
12
lib/foundation/fabro-api/tests/manifest_args_round_trip.rs
Normal file
12
lib/foundation/fabro-api/tests/manifest_args_round_trip.rs
Normal file
|
|
@ -0,0 +1,12 @@
|
|||
use fabro_api::types;
|
||||
use serde_json::json;
|
||||
|
||||
#[test]
|
||||
fn removed_docker_image_manifest_arg_is_ignored() {
|
||||
let args: types::ManifestArgs = serde_json::from_value(json!({
|
||||
"docker_image": "ghcr.io/fabro/custom:latest"
|
||||
}))
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(serde_json::to_value(args).unwrap(), json!({}));
|
||||
}
|
||||
|
|
@ -24,10 +24,6 @@ export interface ManifestArgs {
|
|||
* Named environment slug to select for the run.
|
||||
*/
|
||||
'environment'?: string;
|
||||
/**
|
||||
* Per-run environment image override.
|
||||
*/
|
||||
'docker_image'?: string;
|
||||
'verbose'?: boolean;
|
||||
'dry_run'?: boolean;
|
||||
'auto_approve'?: boolean;
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue