mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-14 23:22:51 +00:00
Fix CLI RunIntent producer CI failures
This commit is contained in:
parent
2507a0075f
commit
afe1133878
12 changed files with 72 additions and 43 deletions
|
|
@ -1,6 +1,7 @@
|
|||
use std::path::Path;
|
||||
|
||||
use anyhow::{Context as _, anyhow, bail};
|
||||
use fabro_config::project;
|
||||
use fabro_server::manifest_validation;
|
||||
use fabro_types::settings::run::EnvironmentProvider;
|
||||
use fabro_types::{DirtyStatus, RunId, RunIntent, RunTarget};
|
||||
|
|
@ -68,8 +69,7 @@ pub(crate) async fn create_run(
|
|||
ctx.base_config_path(),
|
||||
*ctx.run_settings_key_presence(),
|
||||
);
|
||||
let project_config =
|
||||
fabro_config::project::discover_project_config(&package.workflow_location().dir)?;
|
||||
let project_config = project::discover_project_config(&package.workflow_location().dir)?;
|
||||
if let Some(path) = project_config.as_deref() {
|
||||
warn_untransmitted_settings(
|
||||
ctx,
|
||||
|
|
|
|||
|
|
@ -76,6 +76,10 @@ fn current_dir_or_dot() -> PathBuf {
|
|||
std::env::current_dir().unwrap_or_else(|_| PathBuf::from("."))
|
||||
}
|
||||
|
||||
#[expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "CLI argument preparation synchronously reads one local goal file before submission"
|
||||
)]
|
||||
pub(super) fn prepare_intent_overrides(
|
||||
args: &RunArgs,
|
||||
cwd: &Path,
|
||||
|
|
@ -176,11 +180,12 @@ pub(crate) fn preflight_args_overrides(args: &PreflightArgs) -> Result<ManifestS
|
|||
)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::args::{InputOverrideArgs, ServerTargetArgs};
|
||||
|
||||
fn run_args() -> RunArgs {
|
||||
RunArgs {
|
||||
target: crate::args::ServerTargetArgs::default(),
|
||||
inputs: crate::args::InputOverrideArgs::default(),
|
||||
target: ServerTargetArgs::default(),
|
||||
inputs: InputOverrideArgs::default(),
|
||||
workflow: Some(PathBuf::from("workflow.fabro")),
|
||||
dry_run: false,
|
||||
auto_approve: false,
|
||||
|
|
|
|||
|
|
@ -364,6 +364,8 @@ fn attach_replays_completed_detached_run() {
|
|||
"--dry-run",
|
||||
"--auto-approve",
|
||||
"--detach",
|
||||
"--environment",
|
||||
"local",
|
||||
workflow.to_str().unwrap(),
|
||||
])
|
||||
.assert()
|
||||
|
|
@ -884,7 +886,6 @@ fn attach_json_errors_without_prompting_for_human_input() {
|
|||
}
|
||||
}
|
||||
},
|
||||
"manifest_blob": "[BLOB_HASH]",
|
||||
"provenance": {
|
||||
"client": {
|
||||
"name": "fabro-cli",
|
||||
|
|
@ -1014,10 +1015,15 @@ fn attach_json_errors_without_prompting_for_human_input() {
|
|||
},
|
||||
"source_directory": "[TEMP_DIR]",
|
||||
"spec_blob": "[BLOB_HASH]",
|
||||
"target": {
|
||||
"kind": "folder",
|
||||
"path": "[TEMP_DIR]"
|
||||
},
|
||||
"title": "Wait for approval",
|
||||
"web_url": "http://localhost:3000/runs/[ULID]",
|
||||
"workflow_slug": "human-gate",
|
||||
"workflow_source": "digraph HumanGate {/n graph [goal=\"Wait for approval\"]/n start [shape=Mdiamond, label=\"Start\"]/n exit [shape=Msquare, label=\"Exit\"]/n approve [shape=hexagon, label=\"Approve?\"]/n ship [shape=parallelogram, script=\"echo shipped\"]/n revise [shape=parallelogram, script=\"echo revised\"]/n start -> approve/n approve -> ship [label=\"[A] Approve\"]/n approve -> revise [label=\"[R] Revise\"]/n ship -> exit/n revise -> exit/n}/n"
|
||||
"workflow_source": "digraph HumanGate {/n graph [goal=\"Wait for approval\"]/n start [shape=Mdiamond, label=\"Start\"]/n exit [shape=Msquare, label=\"Exit\"]/n approve [shape=hexagon, label=\"Approve?\"]/n ship [shape=parallelogram, script=\"echo shipped\"]/n revise [shape=parallelogram, script=\"echo revised\"]/n start -> approve/n approve -> ship [label=\"[A] Approve\"]/n approve -> revise [label=\"[R] Revise\"]/n ship -> exit/n revise -> exit/n}/n",
|
||||
"workflow_version_id": "fc1611d3be115f2db472e4ac05a5034f449743089259566b18f204ff961a0c18"
|
||||
},
|
||||
"run_id": "[ULID]",
|
||||
"ts": "[TIMESTAMP]"
|
||||
|
|
|
|||
|
|
@ -1,3 +1,8 @@
|
|||
#![expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "integration tests stage temporary workflow and Git fixtures with synchronous APIs"
|
||||
)]
|
||||
|
||||
use std::sync::atomic::{AtomicUsize, Ordering};
|
||||
use std::sync::{Arc, Mutex};
|
||||
|
||||
|
|
@ -63,12 +68,19 @@ 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()).unwrap();
|
||||
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().unwrap() }).to_string())
|
||||
.body(
|
||||
json!({
|
||||
"workflow_version_id": version
|
||||
.id()
|
||||
.expect("mocked workflow version should have a valid ID")
|
||||
})
|
||||
.to_string(),
|
||||
)
|
||||
.build()
|
||||
});
|
||||
})
|
||||
|
|
@ -83,10 +95,10 @@ fn mock_intent_create<'a>(
|
|||
server.mock(|when, then| {
|
||||
when.method("POST").path("/api/v1/runs");
|
||||
then.respond_with(move |request| {
|
||||
requests
|
||||
.lock()
|
||||
.unwrap()
|
||||
.push(serde_json::from_slice(request.body_ref()).unwrap());
|
||||
requests.lock().unwrap().push(
|
||||
serde_json::from_slice(request.body_ref())
|
||||
.expect("run-intent request body should be valid JSON"),
|
||||
);
|
||||
HttpMockResponse::builder()
|
||||
.status(201)
|
||||
.header("content-type", "application/json")
|
||||
|
|
@ -98,19 +110,19 @@ fn mock_intent_create<'a>(
|
|||
|
||||
fn write_workflow(root: &std::path::Path, directory: &str, graph_name: &str) -> std::path::PathBuf {
|
||||
let directory = root.join(directory);
|
||||
std::fs::create_dir_all(&directory).unwrap();
|
||||
std::fs::create_dir_all(&directory).expect("workflow fixture directory should be created");
|
||||
std::fs::write(
|
||||
directory.join("workflow.toml"),
|
||||
"_version = 1\n\n[workflow]\ngraph = \"workflow.fabro\"\n",
|
||||
)
|
||||
.unwrap();
|
||||
.expect("workflow fixture manifest should be written");
|
||||
std::fs::write(
|
||||
directory.join("workflow.fabro"),
|
||||
format!(
|
||||
"digraph {graph_name} {{ start [shape=Mdiamond] exit [shape=Msquare] start -> exit }}"
|
||||
),
|
||||
)
|
||||
.unwrap();
|
||||
.expect("workflow fixture graph should be written");
|
||||
directory.join("workflow.toml")
|
||||
}
|
||||
|
||||
|
|
@ -123,13 +135,16 @@ fn run_git(path: &std::path::Path, args: &[&str]) -> String {
|
|||
.args(args)
|
||||
.current_dir(path)
|
||||
.output()
|
||||
.unwrap();
|
||||
.expect("Git fixture command should execute");
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"git {args:?} failed: {}",
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
String::from_utf8(output.stdout).unwrap().trim().to_string()
|
||||
String::from_utf8(output.stdout)
|
||||
.expect("Git fixture output should be UTF-8")
|
||||
.trim()
|
||||
.to_string()
|
||||
}
|
||||
|
||||
fn init_git_repository(path: &std::path::Path) {
|
||||
|
|
@ -540,7 +555,7 @@ provider = "local"
|
|||
})
|
||||
);
|
||||
assert_eq!(intent["environment_id"], "local");
|
||||
assert_eq!(intent["parent_id"], parent_id.to_string());
|
||||
assert_eq!(intent["parent_id"], parent_id.clone());
|
||||
assert_eq!(intent["goal"], "Goal read from caller cwd");
|
||||
assert_eq!(intent["args"]["model"], "gpt-5");
|
||||
assert_eq!(intent["args"]["provider"], "openai");
|
||||
|
|
@ -1532,12 +1547,12 @@ fn create_uses_workflow_owned_pull_request_settings_only() {
|
|||
std::fs::create_dir_all(project.path().join(".fabro")).unwrap();
|
||||
std::fs::write(
|
||||
project.path().join(".fabro/project.toml"),
|
||||
r#"_version = 1
|
||||
r"_version = 1
|
||||
|
||||
[run.pull_request]
|
||||
enabled = true
|
||||
draft = false
|
||||
"#,
|
||||
",
|
||||
)
|
||||
.unwrap();
|
||||
let workflow = write_workflow(project.path(), "workflow", "PullRequestAuthority");
|
||||
|
|
|
|||
|
|
@ -168,7 +168,7 @@ fn dump_exports_blob_refs_and_artifacts_together() {
|
|||
)
|
||||
.unwrap();
|
||||
fs::write(
|
||||
workspace_dir.join("run.toml"),
|
||||
workspace_dir.join("workflow.toml"),
|
||||
r#"_version = 1
|
||||
|
||||
[workflow]
|
||||
|
|
@ -189,7 +189,7 @@ include = ["assets/**"]
|
|||
let mut run_cmd = context.run_cmd();
|
||||
run_cmd.current_dir(&workspace_dir);
|
||||
run_cmd.timeout(Duration::from_secs(30));
|
||||
run_cmd.args(["--environment", "local", "run.toml"]);
|
||||
run_cmd.args(["--environment", "local", "workflow.toml"]);
|
||||
let run_output = run_cmd.output().expect("command should execute");
|
||||
assert!(
|
||||
run_output.status.success(),
|
||||
|
|
|
|||
|
|
@ -59,13 +59,18 @@ 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()).unwrap();
|
||||
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().unwrap() }).to_string(),
|
||||
serde_json::json!({
|
||||
"workflow_version_id": version
|
||||
.id()
|
||||
.expect("mocked workflow version should have a valid ID")
|
||||
})
|
||||
.to_string(),
|
||||
)
|
||||
.build()
|
||||
});
|
||||
|
|
@ -808,7 +813,7 @@ fn local_foreground_run_prints_artifact_paths_from_server_artifact_list() {
|
|||
"#,
|
||||
);
|
||||
context.write_temp(
|
||||
"artifact-summary/run.toml",
|
||||
"artifact-summary/workflow.toml",
|
||||
r#"_version = 1
|
||||
|
||||
[workflow]
|
||||
|
|
@ -835,7 +840,7 @@ include = ["assets/**"]
|
|||
"local",
|
||||
"--provider",
|
||||
"openai",
|
||||
"run.toml",
|
||||
"workflow.toml",
|
||||
])
|
||||
.output()
|
||||
.expect("command should execute");
|
||||
|
|
|
|||
|
|
@ -397,7 +397,7 @@ pub(crate) fn setup_local_sandbox_run(context: &TestContext) -> WorkspaceRunSetu
|
|||
"#,
|
||||
);
|
||||
write_text_file(
|
||||
&workspace_dir.join("run.toml"),
|
||||
&workspace_dir.join("workflow.toml"),
|
||||
r#"_version = 1
|
||||
|
||||
[workflow]
|
||||
|
|
@ -412,7 +412,7 @@ id = "local"
|
|||
"#,
|
||||
);
|
||||
|
||||
let run = run_local_workflow(context, &workspace_dir, "run.toml");
|
||||
let run = run_local_workflow(context, &workspace_dir, "workflow.toml");
|
||||
assert!(run_state(&run.run_dir).sandbox.is_some());
|
||||
|
||||
WorkspaceRunSetup { run, workspace_dir }
|
||||
|
|
|
|||
|
|
@ -221,7 +221,7 @@ fn dry_run_detach_attach_works_with_default_run_lookup() {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn completed_run_can_be_attached_by_workflow_slug() {
|
||||
fn completed_run_can_be_attached_by_entrypoint_slug() {
|
||||
let context = test_context!();
|
||||
context.ensure_home_server_auth_methods();
|
||||
let project = tempfile::tempdir().unwrap();
|
||||
|
|
@ -258,7 +258,7 @@ digraph BarBaz {
|
|||
context
|
||||
.command()
|
||||
.current_dir(project.path())
|
||||
.args(["start", "sluggy"])
|
||||
.args(["start", "workflow"])
|
||||
.assert()
|
||||
.success();
|
||||
context
|
||||
|
|
@ -271,7 +271,7 @@ digraph BarBaz {
|
|||
context
|
||||
.command()
|
||||
.current_dir(project.path())
|
||||
.args(["attach", "sluggy"])
|
||||
.args(["attach", "workflow"])
|
||||
.timeout(std::time::Duration::from_secs(10))
|
||||
.assert()
|
||||
.success();
|
||||
|
|
|
|||
|
|
@ -175,7 +175,7 @@ fn acp_artifacts_are_listed_when_touched_file_mtime_precedes_attempt_start() {
|
|||
),
|
||||
);
|
||||
context.write_temp(
|
||||
"run.toml",
|
||||
"workflow.toml",
|
||||
r#"_version = 1
|
||||
|
||||
[workflow]
|
||||
|
|
@ -196,7 +196,7 @@ include = ["verification-artifacts/**"]
|
|||
context
|
||||
.run_cmd()
|
||||
.args(["--auto-approve", "--environment", "local"])
|
||||
.arg(context.temp_dir.join("run.toml"))
|
||||
.arg(context.temp_dir.join("workflow.toml"))
|
||||
.assert()
|
||||
.success();
|
||||
|
||||
|
|
|
|||
|
|
@ -29,7 +29,7 @@ fn unchanged_matching_artifact_is_captured_once_across_stages() {
|
|||
"#,
|
||||
);
|
||||
context.write_temp(
|
||||
"run.toml",
|
||||
"workflow.toml",
|
||||
r#"_version = 1
|
||||
|
||||
[workflow]
|
||||
|
|
@ -50,7 +50,7 @@ include = ["assets/**"]
|
|||
context
|
||||
.run_cmd()
|
||||
.args(["--auto-approve", "--environment", "local"])
|
||||
.arg(context.temp_dir.join("run.toml"))
|
||||
.arg(context.temp_dir.join("workflow.toml"))
|
||||
.assert()
|
||||
.success();
|
||||
|
||||
|
|
|
|||
|
|
@ -18,7 +18,7 @@ use fabro_api::types::{
|
|||
BoardColumn, ManifestConfigType, ManifestGoalType, RunIntent, RunManifest, SubmitAnswerRequest,
|
||||
UpdateRunParentRequest, UpdateRunRequest,
|
||||
};
|
||||
use fabro_config::{CliLayer, RunLayer, Storage};
|
||||
use fabro_config::{CliLayer, RunLayer, Storage, project};
|
||||
use fabro_environment::{DEFAULT_ENVIRONMENT_ID, EnvironmentId};
|
||||
use fabro_interview::AnswerSubmission;
|
||||
use fabro_llm::client::Client as LlmClient;
|
||||
|
|
@ -704,7 +704,7 @@ pub(crate) async fn create_run_from_intent(
|
|||
});
|
||||
|
||||
let entrypoint = lowered.entrypoint.clone();
|
||||
let workflow_slug = fabro_config::project::workflow_slug_from_path(entrypoint.as_path());
|
||||
let workflow_slug = project::workflow_slug_from_path(entrypoint.as_path());
|
||||
let raw_compiler_input = RawRunCompilerInput {
|
||||
workflow_bundle: lowered.workflow_bundle,
|
||||
entrypoint: lowered.entrypoint,
|
||||
|
|
|
|||
|
|
@ -2337,6 +2337,7 @@ mod tests {
|
|||
|
||||
use chrono::Duration as ChronoDuration;
|
||||
use fabro_types::WorkflowPath;
|
||||
use fabro_types::settings::run::EnvironmentProvider;
|
||||
use fabro_util::exit;
|
||||
use httpmock::Method::{GET, POST};
|
||||
use httpmock::{HttpMockResponse, MockServer};
|
||||
|
|
@ -2456,10 +2457,7 @@ mod tests {
|
|||
|
||||
mock.assert_async().await;
|
||||
assert_eq!(environment.id.as_str(), "local");
|
||||
assert_eq!(
|
||||
environment.settings.provider,
|
||||
fabro_types::settings::run::EnvironmentProvider::Local
|
||||
);
|
||||
assert_eq!(environment.settings.provider, EnvironmentProvider::Local);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue