From ee1502f7935e1f18c4de789964783f8ada3850c3 Mon Sep 17 00:00:00 2001 From: "fabro-sh-0530[bot]" <281434857+fabro-sh-0530[bot]@users.noreply.github.com> Date: Wed, 27 May 2026 20:14:56 -0400 Subject: [PATCH] Add automation run materialization core and shared run creation helper (#441) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Automation-triggered runs need to share the same run creation pipeline as `POST /runs`. This PR lays the core infrastructure: a `create_run_from_manifest` helper that the HTTP handler and the upcoming automation scheduler can both call, plus a `AutomationRunMaterializer` trait with a production implementation that clones a GitHub repo and builds a `RunManifest` from it. ### Plan Summary - Extract the body of `handler/runs.rs::create_run` into a crate-private `create_run_from_manifest(state, CreateRunFromManifestRequest)` helper; `POST /runs` calls it with `automation: None`, preserving existing behavior. - Add `AutomationRunMaterializeInput/Materialized/Error` types and the `AutomationRunMaterializer` trait (`automation_materializer.rs`). - Implement `ProductionAutomationRunMaterializer`: validates `owner/repo` slug, shallow-clones via `tokio::process::Command` argv (never shell strings), sets `GIT_TERMINAL_PROMPT=0`, enforces per-operation timeouts, redacts credentials from error text, resolves the workflow with `fabro_config::project::WorkflowLocation::resolve`, and builds a `RunManifest` via `fabro_manifest::build_run_manifest`. - Add `TestAutomationRunMaterializer` (gated on `test` or `test-support`) for fake injection in route tests without network access. - Wire the materializer override into `AppState` and `AppStateConfig` behind `#[cfg(any(test, feature = "test-support"))]`; expose via `TestAppStateBuilder::automation_materializer`. - Move `async-trait` from `[dev-dependencies]` to `[dependencies]` in `fabro-server` since the trait is now in production code. ## What changed and why **`automation_materializer.rs` (new)** — Core of this PR. The `GitCommandPlan` builder keeps all git invocations as argv slices so there is no shell injection surface. Credentials are injected exclusively via `GIT_CONFIG_VALUE_0` (the `extraheader` mechanism), never embedded in the clone URL, so they cannot appear in run metadata or error messages. The `redact_git_output` function scrubs the raw token, the Base64-encoded form, and the full `AUTHORIZATION` header value from any error string before it surfaces. **`create_run_from_manifest`** — The extracted helper accepts an optional `AutomationRef` which is forwarded into `create_input.automation` so the store can persist automation provenance on the run. The `POST /runs` code path passes `None`, leaving existing API behavior identical. **Test injection** — `TestAutomationRunMaterializer` captures every `AutomationRunMaterializeInput` it receives and returns a caller-controlled `Result`, letting route tests assert what inputs the scheduler would pass without touching GitHub. ```mermaid flowchart TB A["POST /runs\n(HTTP handler)"] -->|automation: None| H["create_run_from_manifest"] S["Automation scheduler\n(future issue)"] -->|automation: Some(ref)| H H --> DB[(Run store)] M["AutomationRunMaterializer\n(trait)"] -->|produces RunManifest| S M -- production --> P["ProductionAutomationRunMaterializer\n(git clone → manifest build)"] M -- test --> T["TestAutomationRunMaterializer\n(captures input, returns fixture)"] ``` ### Fabro Details
Ran 8 stages in 72m 56s for $36.55 | Stage | Duration | Cost | Retries | |---|---|---|---| | start | 0s | – | 0 | | toolchain | 2s | – | 0 | | preflight_compile | 2m 12s | – | 0 | | preflight_lint | 2m 27s | – | 0 | | implement | 30m 41s | $23.10 | 0 | | simplify_opus | 19m 19s | $9.12 | 0 | | simplify_gpt | 7m 53s | $4.33 | 0 | | verify | 9m 20s | – | 0 | | **Total** | **72m 56s** | **$36.55** | **0** |
Ran ImplementPlan.fabro (11 nodes and 14 edges) ```dot digraph ImplementPlan { graph [ goal="Implement and simplify", model_stylesheet=" * { model: claude-opus-4-7; } " ] rankdir=LR start [shape=Mdiamond, label="Start"] exit [shape=Msquare, label="Exit"] toolchain [label="Toolchain", shape=parallelogram, script="command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1", max_retries=0] preflight_compile [label="Preflight Compile", shape=parallelogram, script="cargo check -q --workspace 2>&1", max_retries=0] preflight_lint [label="Preflight Lint", shape=parallelogram, script="cargo +nightly-2026-04-14 clippy -q --workspace --all-targets -- -D warnings 2>&1", max_retries=0] fix_lints [label="Fix Lints", prompt="The preflight lint step failed. Read the build output from context and fix all clippy lint warnings.", max_visits=3] implement [label="Implement", prompt="Read the plan file referenced in the goal and implement every step. Make all the code changes described in the plan. Use red/green TDD.", model="gpt-55", reasoning_effort="xhigh"] simplify_opus [label="Simplify (Opus)", prompt="@prompts/simplify.md"] simplify_gpt [label="Simplify (GPT-55)", prompt="@prompts/simplify.md", model="gpt-55"] verify [label="Verify", shape=parallelogram, script="git fetch origin main 2>&1 && git merge --no-edit --no-stat origin/main 2>&1 && cargo +nightly-2026-04-14 fmt --all 2>&1 && cargo dev docs refresh 2>&1 && cargo +nightly-2026-04-14 fmt --check --all 2>&1 && { command -v rg >/dev/null 2>&1 || { echo 'rg is required for verify'; exit 127; }; } && ! rg -n 'AuthMode::Disabled|RunAuthMethod|RunSubjectProvenance|\bActorRef\b|\bActorKind\b|AuthenticatedSubject|AuthenticatedService|AuthorizeRunScoped|AuthorizeRunBlob|AuthorizeStageArtifact|AuthorizeCommandLog|auth_method\s*==\s*\"disabled\"' lib/crates apps lib/packages docs/public/api-reference/fabro-api.yaml 2>&1 && cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings 2>&1 && cargo nextest run --workspace --status-level slow --profile ci 2>&1 && cargo dev docs check 2>&1 && bun install --frozen-lockfile 2>&1 && (cd apps/fabro-web && bun run typecheck) 2>&1 && (cd apps/fabro-web && bun run test) 2>&1 && (cd lib/packages/fabro-api-client && bun run typecheck) 2>&1 && cargo dev build -- -p fabro-cli --release 2>&1", goal_gate=true, retry_target="fixup"] fixup [label="Fixup", prompt="The verify step failed. Read the build output from context and fix all format, clippy, Rust test, docs, TypeScript typecheck/test, and build failures.", max_visits=3] start -> toolchain toolchain -> preflight_compile [condition="outcome=succeeded"] toolchain -> exit preflight_compile -> preflight_lint [condition="outcome=succeeded"] preflight_compile -> exit preflight_lint -> implement [condition="outcome=succeeded"] preflight_lint -> fix_lints fix_lints -> preflight_lint implement -> simplify_opus -> simplify_gpt -> verify verify -> exit [condition="outcome=succeeded"] verify -> fixup fixup -> verify } ```
⚒️ Generated with [Fabro](https://fabro.sh) --------- Co-authored-by: Fabro --- lib/crates/fabro-api/build.rs | 1 + lib/crates/fabro-api/src/lib.rs | 53 +- .../fabro-api/tests/run_summary_round_trip.rs | 7 +- lib/crates/fabro-server/Cargo.toml | 2 +- .../src/automation_materializer.rs | 797 ++++++++++++++++++ lib/crates/fabro-server/src/lib.rs | 5 + lib/crates/fabro-server/src/serve.rs | 2 + lib/crates/fabro-server/src/server.rs | 65 +- .../fabro-server/src/server/handler/mod.rs | 2 +- .../fabro-server/src/server/handler/runs.rs | 54 +- lib/crates/fabro-server/src/server/tests.rs | 156 ++++ lib/crates/fabro-server/src/test_support.rs | 10 + 12 files changed, 1104 insertions(+), 50 deletions(-) create mode 100644 lib/crates/fabro-server/src/automation_materializer.rs diff --git a/lib/crates/fabro-api/build.rs b/lib/crates/fabro-api/build.rs index 185435b66..d7ddfbba2 100644 --- a/lib/crates/fabro-api/build.rs +++ b/lib/crates/fabro-api/build.rs @@ -632,6 +632,7 @@ fn main() { ("SandboxTimestamps", "fabro_types::SandboxTimestamps", &[]), ("AskFabro", "fabro_types::AskFabro", &[]), ("Automation", "fabro_automation::Automation", &[]), + ("AutomationRef", "fabro_types::AutomationRef", &[]), ("AutomationTarget", "fabro_automation::AutomationTarget", &[ ]), ( diff --git a/lib/crates/fabro-api/src/lib.rs b/lib/crates/fabro-api/src/lib.rs index a3dcd6867..6c0b06906 100644 --- a/lib/crates/fabro-api/src/lib.rs +++ b/lib/crates/fabro-api/src/lib.rs @@ -40,32 +40,33 @@ pub mod types { pub use fabro_types::{ ActivatedSkill, AgentMcpToolSummary, AgentSkillActivationSource, AgentSkillSummary, AgentToolCategory, AgentToolSource, AgentToolSummary, AgentToolsAvailableProps, AskFabro, - AuthMethod, BilledTokenCounts, CommandTermination, Conclusion, CreateVariableRequest, - DiffStats, DiffSummary, DirtyStatus, EventEnvelope, ExecOutputTail, FailureCategory, - FailureDetail, FailureSignature, GitContext, IdpIdentity, IntegrationConnectionKind, - IntegrationConnectionState, IntegrationConnectionStatus, IntegrationProvider, - IntegrationStatus, InterviewOption, InterviewQuestionRecord, McpServerProjection, - McpServerStatus, PairId, PairMessageId, PairMessageRecord, PairMessageRequest, PairRecord, - PairStartRequest, PairStatus, PairTarget, PairTranscriptEntry, PairTranscriptResponse, - PendingInterviewRecord, PermissionLevel, PreRunPushOutcome, Principal, PullRequest, - PullRequestDetails, PullRequestDetailsStatus, PullRequestDetailsUnavailableReason, - PullRequestLink, PullRequestMeta, PullRequestResponse, QuestionType, RepositoryRef, Run, - RunApproval, RunApprovalState, RunClientProvenance, RunEvent, RunEventDetailContentKind, - RunEventDetailResponse, RunFailure, RunPairStatusResponse, RunProjection, RunProvenance, - RunRunnableSource, RunSandbox, RunSandboxFailure, RunSandboxInstance, RunSandboxKind, - RunSandboxPlan, RunSandboxRuntime, RunServerProvenance, RunSize, SandboxDetails, - SandboxInfo, SandboxListMeta, SandboxListResponse, SandboxNetwork, SandboxNetworkPolicy, - SandboxNetworkPolicyMode, SandboxProviderKind, SandboxProviderLookupError, - SandboxResources, SandboxService, SandboxServiceListResponse, SandboxState, - SandboxTimestamps, SecretMetadata, SecretType, ServerSettings, SessionDetail, SessionId, - SessionMessage, SessionRecord, SessionStatus, SessionSummary, SessionTurn, - SkillsProjection, StageCompletion, StageContextWindow, StageContextWindowBreakdownItem, - StageContextWindowCategory, StageContextWindowCountMethod, StageContextWindowProjection, - StageContextWindowStaleness, StageContextWindowUnavailableReason, - StageContextWindowWarning, StageHandler, StageModelUsage, StageOutcome, StageProjection, - StageState, SubAgentProjection, SubAgentStatus, SystemActorKind, SystemIntegrationStatus, - SystemIntegrationsResponse, TodoListProjection, TurnId, UpdateVariableRequest, - UserPrincipal, Variable, VariableListResponse, WorkflowSettings, + AuthMethod, AutomationRef, BilledTokenCounts, CommandTermination, Conclusion, + CreateVariableRequest, DiffStats, DiffSummary, DirtyStatus, EventEnvelope, ExecOutputTail, + FailureCategory, FailureDetail, FailureSignature, GitContext, IdpIdentity, + IntegrationConnectionKind, IntegrationConnectionState, IntegrationConnectionStatus, + IntegrationProvider, IntegrationStatus, InterviewOption, InterviewQuestionRecord, + McpServerProjection, McpServerStatus, PairId, PairMessageId, PairMessageRecord, + PairMessageRequest, PairRecord, PairStartRequest, PairStatus, PairTarget, + PairTranscriptEntry, PairTranscriptResponse, PendingInterviewRecord, PermissionLevel, + PreRunPushOutcome, Principal, PullRequest, PullRequestDetails, PullRequestDetailsStatus, + PullRequestDetailsUnavailableReason, PullRequestLink, PullRequestMeta, PullRequestResponse, + QuestionType, RepositoryRef, Run, RunApproval, RunApprovalState, RunClientProvenance, + RunEvent, RunEventDetailContentKind, RunEventDetailResponse, RunFailure, + RunPairStatusResponse, RunProjection, RunProvenance, RunRunnableSource, RunSandbox, + RunSandboxFailure, RunSandboxInstance, RunSandboxKind, RunSandboxPlan, RunSandboxRuntime, + RunServerProvenance, RunSize, SandboxDetails, SandboxInfo, SandboxListMeta, + SandboxListResponse, SandboxNetwork, SandboxNetworkPolicy, SandboxNetworkPolicyMode, + SandboxProviderKind, SandboxProviderLookupError, SandboxResources, SandboxService, + SandboxServiceListResponse, SandboxState, SandboxTimestamps, SecretMetadata, SecretType, + ServerSettings, SessionDetail, SessionId, SessionMessage, SessionRecord, SessionStatus, + SessionSummary, SessionTurn, SkillsProjection, StageCompletion, StageContextWindow, + StageContextWindowBreakdownItem, StageContextWindowCategory, StageContextWindowCountMethod, + StageContextWindowProjection, StageContextWindowStaleness, + StageContextWindowUnavailableReason, StageContextWindowWarning, StageHandler, + StageModelUsage, StageOutcome, StageProjection, StageState, SubAgentProjection, + SubAgentStatus, SystemActorKind, SystemIntegrationStatus, SystemIntegrationsResponse, + TodoListProjection, TurnId, UpdateVariableRequest, UserPrincipal, Variable, + VariableListResponse, WorkflowSettings, }; pub use crate::generated::types::*; diff --git a/lib/crates/fabro-api/tests/run_summary_round_trip.rs b/lib/crates/fabro-api/tests/run_summary_round_trip.rs index 0c5a70fb5..798448941 100644 --- a/lib/crates/fabro-api/tests/run_summary_round_trip.rs +++ b/lib/crates/fabro-api/tests/run_summary_round_trip.rs @@ -3,9 +3,9 @@ use std::collections::HashMap; use chrono::{TimeZone, Utc}; use fabro_api::types::{ - RepositoryRef as ApiRepositoryRef, Run as ApiRun, RunApproval as ApiRunApproval, - RunApprovalState as ApiRunApprovalState, RunRunnableSource as ApiRunRunnableSource, - RunSize as ApiRunSize, + AutomationRef as ApiAutomationRef, RepositoryRef as ApiRepositoryRef, Run as ApiRun, + RunApproval as ApiRunApproval, RunApprovalState as ApiRunApprovalState, + RunRunnableSource as ApiRunRunnableSource, RunSize as ApiRunSize, }; use fabro_types::status::{RunStatus, SuccessReason}; use fabro_types::{ @@ -24,6 +24,7 @@ fn run_summary_reuses_domain_types() { assert_same_type::(); assert_same_type::(); assert_same_type::(); + assert_same_type::(); } #[test] diff --git a/lib/crates/fabro-server/Cargo.toml b/lib/crates/fabro-server/Cargo.toml index 89611b971..2b9568ce1 100644 --- a/lib/crates/fabro-server/Cargo.toml +++ b/lib/crates/fabro-server/Cargo.toml @@ -69,6 +69,7 @@ serde.workspace = true serde_json.workspace = true serde_yaml = "0.9" anyhow.workspace = true +async-trait.workspace = true clap.workspace = true toml.workspace = true toml_edit.workspace = true @@ -110,7 +111,6 @@ http-body-util = "0.1" httpmock = "0.8" serde_yaml = "0.9" tracing-subscriber.workspace = true -async-trait.workspace = true tokio-util.workspace = true fabro-macros = { path = "../fabro-macros" } fabro-sandbox = { path = "../fabro-sandbox", features = ["test-support"] } diff --git a/lib/crates/fabro-server/src/automation_materializer.rs b/lib/crates/fabro-server/src/automation_materializer.rs new file mode 100644 index 000000000..74c9c2677 --- /dev/null +++ b/lib/crates/fabro-server/src/automation_materializer.rs @@ -0,0 +1,797 @@ +use std::path::{Path, PathBuf}; +use std::time::Duration; + +use anyhow::Context as _; +use async_trait::async_trait; +use base64::Engine as _; +use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; +use fabro_api::types::RunManifest; +use fabro_automation::{AutomationId, AutomationTarget}; +use fabro_manifest::ManifestBuildInput; +use fabro_types::{DirtyStatus, GitContext, PreRunPushOutcome, RunId}; +use fabro_util::error::collect_chain; +use tokio::process::Command; +use tokio::{fs, task, time}; + +const GIT_CLONE_TIMEOUT: Duration = Duration::from_mins(2); +const GIT_FETCH_TIMEOUT: Duration = Duration::from_mins(1); +const GIT_CHECKOUT_TIMEOUT: Duration = Duration::from_secs(30); +const GIT_REV_PARSE_TIMEOUT: Duration = Duration::from_secs(10); + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct AutomationRunMaterializeInput { + pub automation_id: AutomationId, + pub target: AutomationTarget, + pub run_id: RunId, + pub user_settings_path: PathBuf, + pub temp_root: PathBuf, +} + +#[derive(Debug, Clone)] +pub(crate) struct AutomationRunMaterialized { + pub manifest: RunManifest, + pub submitted_manifest_bytes: Vec, +} + +#[derive(thiserror::Error, Debug, Clone, PartialEq, Eq)] +pub(crate) enum AutomationRunMaterializeError { + #[error("invalid automation target: {0}")] + InvalidTarget(String), + #[error("failed to clone automation repository: {0}")] + CloneFailed(String), + #[error("failed to resolve automation workflow: {0}")] + WorkflowNotFound(String), + #[error("failed to build run manifest: {0}")] + Manifest(String), +} + +#[async_trait] +pub(crate) trait AutomationRunMaterializer: Send + Sync { + async fn materialize( + &self, + input: AutomationRunMaterializeInput, + ) -> Result; +} + +#[derive(Clone)] +pub(crate) struct ProductionAutomationRunMaterializer { + github_credentials: Option, + github_api_base_url: String, + http_client: Option, +} + +impl ProductionAutomationRunMaterializer { + pub(crate) fn new( + github_credentials: Option, + github_api_base_url: String, + http_client: Option, + ) -> Self { + Self { + github_credentials, + github_api_base_url, + http_client, + } + } +} + +#[async_trait] +impl AutomationRunMaterializer for ProductionAutomationRunMaterializer { + async fn materialize( + &self, + input: AutomationRunMaterializeInput, + ) -> Result { + let repo = parse_github_repository_slug(&input.target.repository)?; + fs::create_dir_all(&input.temp_root).await.map_err(|err| { + AutomationRunMaterializeError::CloneFailed(format!( + "failed to create temp root {}: {err}", + input.temp_root.display() + )) + })?; + let temp_dir = tempfile::Builder::new() + .prefix(&format!( + "automation-{}-{}-", + input.automation_id.as_str(), + input.run_id + )) + .tempdir_in(&input.temp_root) + .map_err(|err| { + AutomationRunMaterializeError::CloneFailed(format!( + "failed to create per-run temp directory under {}: {err}", + input.temp_root.display() + )) + })?; + let checkout_dir = temp_dir.path().join("repo"); + let clone_url = github_clone_url(&repo); + let auth = resolve_git_auth_config( + self.github_credentials.as_ref(), + &repo, + &self.github_api_base_url, + self.http_client.clone(), + ) + .await + .map_err(|err| { + AutomationRunMaterializeError::CloneFailed(render_error_chain(err.as_ref())) + })?; + + run_git_plan(build_clone_plan(&clone_url, &checkout_dir, auth.as_ref())).await?; + run_git_plan(build_fetch_ref_plan( + &clone_url, + &checkout_dir, + &input.target.ref_selector, + auth.as_ref(), + )) + .await?; + run_git_plan(build_checkout_ref_plan(&checkout_dir)).await?; + let checked_out_sha = run_git_plan(build_rev_parse_head_plan(&checkout_dir)) + .await + .map(|stdout| String::from_utf8_lossy(&stdout).trim().to_string())?; + + let manifest_input = ManifestFromCheckoutInput { + input, + checkout_dir, + repo, + checked_out_sha: Some(checked_out_sha), + }; + task::spawn_blocking(move || build_manifest_from_checkout(manifest_input)) + .await + .map_err(|err| { + AutomationRunMaterializeError::Manifest(format!( + "manifest build task failed: {err}" + )) + })? + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct GithubRepository { + owner: String, + name: String, +} + +fn parse_github_repository_slug( + value: &str, +) -> Result { + let Some((owner, repo)) = value.split_once('/') else { + return Err(AutomationRunMaterializeError::InvalidTarget(format!( + "repository must be a GitHub owner/repo slug: {value}" + ))); + }; + if repo.contains('/') || !valid_github_owner(owner) || !valid_github_repo(repo) { + return Err(AutomationRunMaterializeError::InvalidTarget(format!( + "repository must be a GitHub owner/repo slug: {value}" + ))); + } + Ok(GithubRepository { + owner: owner.to_string(), + name: repo.to_string(), + }) +} + +fn valid_github_owner(value: &str) -> bool { + if value.is_empty() || value.len() > 39 { + return false; + } + let bytes = value.as_bytes(); + let first = bytes[0]; + let last = bytes[bytes.len() - 1]; + (first.is_ascii_alphanumeric() && last.is_ascii_alphanumeric()) + && bytes + .iter() + .all(|byte| byte.is_ascii_alphanumeric() || *byte == b'-') +} + +fn valid_github_repo(value: &str) -> bool { + !value.is_empty() + && value.len() <= 100 + && value != "." + && value != ".." + && !value.starts_with('.') + && value + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'.' | b'_' | b'-')) +} + +fn github_clone_url(repo: &GithubRepository) -> String { + format!("https://github.com/{}/{}.git", repo.owner, repo.name) +} + +fn github_metadata_url(repo: &GithubRepository) -> String { + format!("https://github.com/{}/{}", repo.owner, repo.name) +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct GitAuthConfig { + extraheader: Option, + sensitive_values: Vec, +} + +impl GitAuthConfig { + fn new(username: Option, password: Option) -> Self { + let Some(password) = password.filter(|value| !value.is_empty()) else { + return Self { + extraheader: None, + sensitive_values: Vec::new(), + }; + }; + let username = username + .filter(|value| !value.is_empty()) + .unwrap_or_else(|| "x-access-token".to_string()); + let encoded_credentials = BASE64_STANDARD.encode(format!("{username}:{password}")); + let extraheader = basic_auth_header_from_encoded(&encoded_credentials); + Self { + sensitive_values: vec![password, encoded_credentials, extraheader.clone()], + extraheader: Some(extraheader), + } + } + + fn git_env(&self, clone_url: &str) -> Vec<(String, String)> { + let Some(extraheader) = self.extraheader.as_ref() else { + return Vec::new(); + }; + vec![ + ("GIT_CONFIG_COUNT".to_string(), "1".to_string()), + ( + "GIT_CONFIG_KEY_0".to_string(), + format!("http.{clone_url}.extraheader"), + ), + ("GIT_CONFIG_VALUE_0".to_string(), extraheader.clone()), + ] + } + + fn sensitive_values(&self) -> &[String] { + &self.sensitive_values + } +} + +async fn resolve_git_auth_config( + credentials: Option<&fabro_github::GitHubCredentials>, + repo: &GithubRepository, + github_api_base_url: &str, + http_client: Option, +) -> anyhow::Result> { + let Some(credentials) = credentials else { + return Ok(None); + }; + let context = match http_client { + Some(client) => { + fabro_github::GitHubContext::with_http_client(credentials, github_api_base_url, client) + } + None => fabro_github::GitHubContext::new(credentials, github_api_base_url), + }; + let (username, password) = + fabro_github::resolve_clone_credentials(&context, &repo.owner, &repo.name).await?; + Ok(Some(GitAuthConfig::new(username, password))) +} + +fn basic_auth_header(username: &str, password: &str) -> String { + basic_auth_header_from_encoded(&BASE64_STANDARD.encode(format!("{username}:{password}"))) +} + +fn basic_auth_header_from_encoded(encoded_credentials: &str) -> String { + format!("AUTHORIZATION: basic {encoded_credentials}") +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct GitCommandPlan { + program: String, + args: Vec, + env: Vec<(String, String)>, + current_dir: Option, + timeout: Duration, + sensitive_values: Vec, +} + +impl GitCommandPlan { + fn new(args: impl IntoIterator>, timeout: Duration) -> Self { + Self { + program: "git".to_string(), + args: args.into_iter().map(Into::into).collect(), + env: vec![("GIT_TERMINAL_PROMPT".to_string(), "0".to_string())], + current_dir: None, + timeout, + sensitive_values: Vec::new(), + } + } + + fn current_dir(mut self, current_dir: impl Into) -> Self { + self.current_dir = Some(current_dir.into()); + self + } + + fn with_auth(mut self, clone_url: &str, auth: Option<&GitAuthConfig>) -> Self { + if let Some(auth) = auth { + self.env.extend(auth.git_env(clone_url)); + self.sensitive_values + .extend(auth.sensitive_values().iter().cloned()); + } + self + } + + fn env_value(&self, name: &str) -> Option<&str> { + self.env + .iter() + .find(|(key, _)| key == name) + .map(|(_, value)| value.as_str()) + } +} + +fn build_clone_plan( + clone_url: &str, + checkout_dir: &Path, + auth: Option<&GitAuthConfig>, +) -> GitCommandPlan { + GitCommandPlan::new( + [ + "clone".to_string(), + "--depth".to_string(), + "1".to_string(), + "--no-checkout".to_string(), + clone_url.to_string(), + checkout_dir.display().to_string(), + ], + GIT_CLONE_TIMEOUT, + ) + .with_auth(clone_url, auth) +} + +fn build_fetch_ref_plan( + clone_url: &str, + checkout_dir: &Path, + ref_selector: &str, + auth: Option<&GitAuthConfig>, +) -> GitCommandPlan { + GitCommandPlan::new( + [ + "fetch".to_string(), + "--depth".to_string(), + "1".to_string(), + "origin".to_string(), + "--".to_string(), + ref_selector.to_string(), + ], + GIT_FETCH_TIMEOUT, + ) + .current_dir(checkout_dir) + .with_auth(clone_url, auth) +} + +fn build_checkout_ref_plan(checkout_dir: &Path) -> GitCommandPlan { + GitCommandPlan::new( + ["checkout", "--force", "--detach", "FETCH_HEAD"], + GIT_CHECKOUT_TIMEOUT, + ) + .current_dir(checkout_dir) +} + +fn build_rev_parse_head_plan(checkout_dir: &Path) -> GitCommandPlan { + GitCommandPlan::new(["rev-parse", "HEAD"], GIT_REV_PARSE_TIMEOUT).current_dir(checkout_dir) +} + +async fn run_git_plan(plan: GitCommandPlan) -> Result, AutomationRunMaterializeError> { + let mut command = Command::new(&plan.program); + command.args(&plan.args); + command.envs(plan.env.iter().map(|(key, value)| (key, value))); + if let Some(current_dir) = plan.current_dir.as_ref() { + command.current_dir(current_dir); + } + command.kill_on_drop(true); + + let output = time::timeout(plan.timeout, command.output()) + .await + .map_err(|_| { + AutomationRunMaterializeError::CloneFailed(format!( + "{} timed out after {}s", + safe_command_label(&plan), + plan.timeout.as_secs() + )) + })? + .map_err(|err| { + AutomationRunMaterializeError::CloneFailed(format!( + "failed to run {}: {err}", + safe_command_label(&plan) + )) + })?; + + if output.status.success() { + return Ok(output.stdout); + } + + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + let mut message = format!( + "{} exited with status {}", + safe_command_label(&plan), + output.status + ); + if !stderr.trim().is_empty() { + message.push_str(": "); + message.push_str(stderr.trim()); + } else if !stdout.trim().is_empty() { + message.push_str(": "); + message.push_str(stdout.trim()); + } + Err(AutomationRunMaterializeError::CloneFailed( + redact_git_output(&message, &plan.sensitive_values), + )) +} + +fn safe_command_label(plan: &GitCommandPlan) -> String { + if plan.args.is_empty() { + plan.program.clone() + } else { + format!("{} {}", plan.program, plan.args.join(" ")) + } +} + +fn redact_git_output(text: &str, sensitive_values: &[String]) -> String { + let mut redacted = fabro_redact::redact_string(text); + for value in sensitive_values + .iter() + .map(String::as_str) + .filter(|value| !value.is_empty()) + { + redacted = redacted.replace(value, "REDACTED"); + } + redacted +} + +fn render_error_chain(error: &(dyn std::error::Error + 'static)) -> String { + collect_chain(error).join(": ") +} + +#[derive(Debug)] +pub(crate) struct ManifestFromCheckoutInput { + input: AutomationRunMaterializeInput, + checkout_dir: PathBuf, + repo: GithubRepository, + checked_out_sha: Option, +} + +fn build_manifest_from_checkout( + args: ManifestFromCheckoutInput, +) -> Result { + let ManifestFromCheckoutInput { + input, + checkout_dir, + repo, + checked_out_sha, + } = args; + let built = fabro_manifest::build_run_manifest(ManifestBuildInput { + workflow: input.target.workflow.as_str().into(), + cwd: checkout_dir, + run_id: Some(input.run_id), + user_settings_path: Some(input.user_settings_path), + ..ManifestBuildInput::default() + }) + .map_err(|err| manifest_build_error(&err))?; + + let mut manifest = built.manifest; + manifest.git = Some(GitContext { + origin_url: github_metadata_url(&repo), + branch: input.target.ref_selector, + sha: checked_out_sha, + dirty: DirtyStatus::Clean, + push_outcome: PreRunPushOutcome::NotAttempted, + }); + let submitted_manifest_bytes = serde_json::to_vec(&manifest) + .with_context(|| { + format!( + "failed to serialize materialized manifest for automation {}", + input.automation_id.as_str() + ) + }) + .map_err(|err| AutomationRunMaterializeError::Manifest(err.to_string()))?; + Ok(AutomationRunMaterialized { + manifest, + submitted_manifest_bytes, + }) +} + +fn manifest_build_error(error: &anyhow::Error) -> AutomationRunMaterializeError { + if error.chain().any(|source| { + source + .downcast_ref::() + .is_some_and(|err| matches!(err, fabro_config::Error::WorkflowNotFound(_))) + }) { + AutomationRunMaterializeError::WorkflowNotFound(render_error_chain(error.as_ref())) + } else { + AutomationRunMaterializeError::Manifest(render_error_chain(error.as_ref())) + } +} + +#[cfg(any(test, feature = "test-support"))] +#[derive(Clone)] +pub struct TestAutomationRunMaterializer { + inner: std::sync::Arc>, +} + +#[cfg(any(test, feature = "test-support"))] +struct TestAutomationRunMaterializerState { + captured_inputs: Vec, + response: Result, +} + +#[cfg(any(test, feature = "test-support"))] +impl TestAutomationRunMaterializer { + pub fn succeed(manifest: RunManifest, submitted_manifest_bytes: Vec) -> Self { + Self::new(Ok(AutomationRunMaterialized { + manifest, + submitted_manifest_bytes, + })) + } + + pub fn fail_invalid_target(message: impl Into) -> Self { + Self::new(Err(AutomationRunMaterializeError::InvalidTarget( + message.into(), + ))) + } + + fn new(response: Result) -> Self { + Self { + inner: std::sync::Arc::new(std::sync::Mutex::new(TestAutomationRunMaterializerState { + captured_inputs: Vec::new(), + response, + })), + } + } + + pub(crate) fn captured_inputs(&self) -> Vec { + self.inner + .lock() + .expect("test automation materializer lock poisoned") + .captured_inputs + .clone() + } + + pub(crate) fn into_materializer(self) -> std::sync::Arc { + std::sync::Arc::new(self) + } +} + +#[cfg(any(test, feature = "test-support"))] +#[async_trait] +impl AutomationRunMaterializer for TestAutomationRunMaterializer { + async fn materialize( + &self, + input: AutomationRunMaterializeInput, + ) -> Result { + let mut guard = self + .inner + .lock() + .expect("test automation materializer lock poisoned"); + guard.captured_inputs.push(input); + guard.response.clone() + } +} + +#[cfg(test)] +mod tests { + #![expect( + clippy::disallowed_methods, + reason = "Materializer unit tests write small temporary workflow fixtures synchronously." + )] + + use std::fs; + use std::path::Path; + + use fabro_automation::{AutomationId, AutomationTarget}; + use fabro_types::{DirtyStatus, PreRunPushOutcome, RunId}; + use tempfile::TempDir; + + use super::*; + + fn target(repository: &str, ref_selector: &str, workflow: &str) -> AutomationTarget { + AutomationTarget { + repository: repository.to_string(), + ref_selector: ref_selector.to_string(), + workflow: workflow.to_string(), + } + } + + #[test] + fn target_repository_urls_are_github_metadata_urls_without_credentials() { + let repo = parse_github_repository_slug("fabro-sh/fabro").expect("slug should parse"); + + assert_eq!(repo.owner, "fabro-sh"); + assert_eq!(repo.name, "fabro"); + assert_eq!( + github_clone_url(&repo), + "https://github.com/fabro-sh/fabro.git" + ); + assert_eq!( + github_metadata_url(&repo), + "https://github.com/fabro-sh/fabro" + ); + assert!(!github_clone_url(&repo).contains('@')); + } + + #[test] + fn target_repository_validation_rejects_non_github_owner_repo_shapes() { + for value in [ + "fabro-sh", + "https://github.com/fabro-sh/fabro", + "fabro-sh/fabro/extra", + "-owner/repo", + "owner/.git", + ] { + let error = parse_github_repository_slug(value).expect_err("invalid slug should fail"); + assert!( + error.to_string().contains("invalid automation target"), + "unexpected error for {value}: {error}" + ); + } + } + + #[test] + fn ref_checkout_command_plans_use_argv_prompt_disable_and_timeouts() { + let repo = parse_github_repository_slug("fabro-sh/fabro").unwrap(); + let clone_url = github_clone_url(&repo); + let temp = TempDir::new().unwrap(); + let checkout_dir = temp.path().join("repo"); + + let clone = build_clone_plan(&clone_url, &checkout_dir, None); + assert_eq!(clone.program, "git"); + assert_eq!(clone.args, vec![ + "clone", + "--depth", + "1", + "--no-checkout", + "https://github.com/fabro-sh/fabro.git", + checkout_dir.to_str().unwrap(), + ]); + assert_eq!(clone.timeout, Duration::from_mins(2)); + assert_eq!(clone.env_value("GIT_TERMINAL_PROMPT"), Some("0")); + + let fetch = build_fetch_ref_plan(&clone_url, &checkout_dir, "feature/materialize", None); + assert_eq!(fetch.args, vec![ + "fetch", + "--depth", + "1", + "origin", + "--", + "feature/materialize", + ]); + assert_eq!(fetch.current_dir.as_deref(), Some(checkout_dir.as_path())); + assert_eq!(fetch.timeout, Duration::from_mins(1)); + assert_eq!(fetch.env_value("GIT_TERMINAL_PROMPT"), Some("0")); + + let checkout = build_checkout_ref_plan(&checkout_dir); + assert_eq!(checkout.args, vec![ + "checkout", + "--force", + "--detach", + "FETCH_HEAD" + ]); + assert_eq!( + checkout.current_dir.as_deref(), + Some(checkout_dir.as_path()) + ); + assert_eq!(checkout.timeout, Duration::from_secs(30)); + assert_eq!(checkout.env_value("GIT_TERMINAL_PROMPT"), Some("0")); + } + + #[test] + fn credential_redaction_removes_tokens_and_basic_auth_headers() { + let secret = "ghu_materializer_secret"; + let basic = basic_auth_header("x-access-token", secret); + let message = format!( + "fatal: could not read Username for https://github.com/fabro-sh/fabro.git; token={secret}; header={basic}" + ); + + let redacted = redact_git_output(&message, &[secret.to_string(), basic.clone()]); + + assert!(!redacted.contains(secret), "token leaked: {redacted}"); + assert!( + !redacted.contains(&basic), + "basic header leaked: {redacted}" + ); + let encoded_secret = BASE64_STANDARD.encode(format!("x-access-token:{secret}")); + assert!( + !redacted.contains(&encoded_secret), + "encoded credential leaked: {redacted}" + ); + assert!( + redacted.contains("REDACTED"), + "expected redaction marker: {redacted}" + ); + } + + #[test] + fn credential_config_env_keeps_clone_url_uncredentialed() { + let repo = parse_github_repository_slug("fabro-sh/fabro").unwrap(); + let clone_url = github_clone_url(&repo); + let auth = GitAuthConfig::new( + Some("x-access-token".to_string()), + Some("ghu_secret".to_string()), + ); + let plan = build_clone_plan(&clone_url, Path::new("/tmp/fabro-checkout"), Some(&auth)); + + assert!( + plan.args + .iter() + .any(|arg| arg == "https://github.com/fabro-sh/fabro.git") + ); + assert!(plan.args.iter().all(|arg| !arg.contains("ghu_secret"))); + assert_eq!(plan.env_value("GIT_CONFIG_COUNT"), Some("1")); + assert_eq!( + plan.env_value("GIT_CONFIG_KEY_0"), + Some("http.https://github.com/fabro-sh/fabro.git.extraheader") + ); + assert!( + plan.env_value("GIT_CONFIG_VALUE_0") + .is_some_and(|value| value.starts_with("AUTHORIZATION: basic ")) + ); + } + + #[test] + fn workflow_path_resolution_builds_manifest_from_checkout_directory() { + let temp = TempDir::new().unwrap(); + let checkout = temp.path().join("checkout"); + let workflow_dir = checkout.join(".fabro/workflows/demo"); + fs::create_dir_all(&workflow_dir).unwrap(); + fs::write(checkout.join(".fabro/project.toml"), "_version = 1\n").unwrap(); + fs::write( + workflow_dir.join("workflow.fabro"), + r#"digraph Demo { graph [goal="Ship automation"] start [shape=Mdiamond] exit [shape=Msquare] start -> exit }"#, + ) + .unwrap(); + fs::write( + workflow_dir.join("workflow.toml"), + "_version = 1\n[workflow]\ngraph = \"workflow.fabro\"\n", + ) + .unwrap(); + let user_settings_path = temp.path().join("settings.toml"); + fs::write(&user_settings_path, "_version = 1\n").unwrap(); + let run_id = RunId::new(); + let repo = parse_github_repository_slug("fabro-sh/fabro").unwrap(); + let sha = "0123456789abcdef0123456789abcdef01234567".to_string(); + + let materialized = build_manifest_from_checkout(ManifestFromCheckoutInput { + input: AutomationRunMaterializeInput { + automation_id: AutomationId::new("nightly").unwrap(), + target: target("fabro-sh/fabro", "main", "demo"), + run_id, + user_settings_path: user_settings_path.clone(), + temp_root: temp.path().to_path_buf(), + }, + checkout_dir: checkout.clone(), + repo, + checked_out_sha: Some(sha.clone()), + }) + .expect("manifest should build from checkout"); + + assert_eq!( + materialized.manifest.run_id.as_deref(), + Some(run_id.to_string().as_str()) + ); + assert_eq!(materialized.manifest.cwd, checkout.display().to_string()); + assert_eq!( + materialized.manifest.target.path, + ".fabro/workflows/demo/workflow.fabro" + ); + assert!( + materialized + .manifest + .configs + .iter() + .any(|config| config.path.as_deref() == Some(user_settings_path.to_str().unwrap())) + ); + let git = materialized + .manifest + .git + .as_ref() + .expect("git context should be set"); + assert_eq!(git.origin_url, "https://github.com/fabro-sh/fabro"); + assert_eq!(git.branch, "main"); + assert_eq!(git.sha.as_deref(), Some(sha.as_str())); + assert_eq!(git.dirty, DirtyStatus::Clean); + assert_eq!(git.push_outcome, PreRunPushOutcome::NotAttempted); + let submitted_manifest: serde_json::Value = + serde_json::from_slice(&materialized.submitted_manifest_bytes) + .expect("submitted bytes should be a manifest"); + assert_eq!( + submitted_manifest, + serde_json::to_value(&materialized.manifest).unwrap() + ); + } +} diff --git a/lib/crates/fabro-server/src/lib.rs b/lib/crates/fabro-server/src/lib.rs index d6b5ffeb4..0e591e26d 100644 --- a/lib/crates/fabro-server/src/lib.rs +++ b/lib/crates/fabro-server/src/lib.rs @@ -9,6 +9,11 @@ )] pub mod auth; +#[allow( + dead_code, + reason = "Automation scheduler wiring will call the materializer; issue #398 adds the shared core first." +)] +mod automation_materializer; mod canonical_host; mod canonical_origin; pub mod csp; diff --git a/lib/crates/fabro-server/src/serve.rs b/lib/crates/fabro-server/src/serve.rs index b2ff5c7ce..b2eb6d91b 100644 --- a/lib/crates/fabro-server/src/serve.rs +++ b/lib/crates/fabro-server/src/serve.rs @@ -814,6 +814,8 @@ where http_client: None, sandbox_provider_registry: None, shutdown: shutdown.clone(), + #[cfg(any(test, feature = "test-support"))] + automation_materializer_override: None, })?; let reconciled = reconcile_incomplete_runs_on_startup(&state).await?; if reconciled > 0 { diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index 44357b2a4..12e05edbc 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -140,6 +140,10 @@ use tracing::{Instrument, debug, error, info, warn}; use ulid::Ulid; use crate::auth::{self, GithubEndpoints, auth_translation_middleware, demo_routing_middleware}; +use crate::automation_materializer::{ + AutomationRunMaterializeError, AutomationRunMaterializeInput, AutomationRunMaterialized, + AutomationRunMaterializer, ProductionAutomationRunMaterializer, +}; use crate::canonical_origin::resolve_canonical_origin; use crate::error::ApiError; use crate::github_webhooks::{ @@ -1009,6 +1013,8 @@ pub struct AppState { session_runtimes: SessionRuntimeManager, artifact_store: ArtifactStore, automation_store: Arc, + #[cfg(any(test, feature = "test-support"))] + automation_materializer_override: Option>, worker_tokens: WorkerTokenKeys, started_at: Instant, resource_sampler: resource_sampler::ResourceSampler, @@ -1049,6 +1055,33 @@ impl AppState { pub(crate) fn automation_store(&self) -> &AutomationStore { &self.automation_store } + + #[allow( + dead_code, + reason = "Automation scheduler wiring will call this after issue #398's materialization core." + )] + pub(crate) async fn materialize_automation_run( + &self, + input: AutomationRunMaterializeInput, + ) -> Result { + #[cfg(any(test, feature = "test-support"))] + if let Some(materializer) = self.automation_materializer_override.as_ref() { + return materializer.materialize(input).await; + } + + let settings = self.server_settings(); + let credentials = self + .github_credentials(&settings.server.integrations.github) + .ok() + .flatten(); + ProductionAutomationRunMaterializer::new( + credentials, + self.github_api_base_url.clone(), + self.http_client.clone(), + ) + .materialize(input) + .await + } } pub(crate) struct AskFabroReadiness { @@ -1130,21 +1163,23 @@ async fn lock_pull_request_create( } pub(crate) struct AppStateConfig { - pub(crate) resolved_settings: ResolvedAppStateSettings, + pub(crate) resolved_settings: ResolvedAppStateSettings, pub(crate) registry_factory_override: Option>, - pub(crate) max_concurrent_runs: usize, - pub(crate) store: Arc, - pub(crate) artifact_store: ArtifactStore, - pub(crate) vault_path: PathBuf, - pub(crate) variables_path: PathBuf, - pub(crate) preloaded_vault: Option, - pub(crate) server_secrets: ServerSecrets, - pub(crate) env_lookup: EnvLookup, - pub(crate) github_api_base_url: Option, - pub(crate) active_config_path: PathBuf, - pub(crate) http_client: Option, + pub(crate) max_concurrent_runs: usize, + pub(crate) store: Arc, + pub(crate) artifact_store: ArtifactStore, + pub(crate) vault_path: PathBuf, + pub(crate) variables_path: PathBuf, + pub(crate) preloaded_vault: Option, + pub(crate) server_secrets: ServerSecrets, + pub(crate) env_lookup: EnvLookup, + pub(crate) github_api_base_url: Option, + pub(crate) active_config_path: PathBuf, + pub(crate) http_client: Option, pub(crate) sandbox_provider_registry: Option, - pub(crate) shutdown: CancellationToken, + pub(crate) shutdown: CancellationToken, + #[cfg(any(test, feature = "test-support"))] + pub(crate) automation_materializer_override: Option>, } #[derive(Clone)] @@ -2195,6 +2230,8 @@ pub(crate) fn build_app_state(config: AppStateConfig) -> anyhow::Result anyhow::Result return ApiError::bad_request(err.to_string()).into_response(), }; let explicit_title_supplied = req.title.is_some(); + Box::pin(create_run_from_manifest( + state, + CreateRunFromManifestRequest { + manifest: req, + submitted_manifest_bytes: body.to_vec(), + explicit_run_id: None, + explicit_title_supplied, + actor, + headers, + automation: None, + }, + )) + .await +} + +pub(crate) struct CreateRunFromManifestRequest { + pub(crate) manifest: RunManifest, + pub(crate) submitted_manifest_bytes: Vec, + pub(crate) explicit_run_id: Option, + pub(crate) explicit_title_supplied: bool, + pub(crate) actor: Principal, + pub(crate) headers: HeaderMap, + pub(crate) automation: Option, +} + +pub(crate) async fn create_run_from_manifest( + state: Arc, + request: CreateRunFromManifestRequest, +) -> Response { + let CreateRunFromManifestRequest { + manifest, + submitted_manifest_bytes, + explicit_run_id, + explicit_title_supplied, + actor, + headers, + automation, + } = request; let manifest_run_defaults = state.manifest_run_defaults(); let manifest_environment_defaults = state.manifest_environment_defaults(); let mut prepared = match run_manifest::prepare_manifest_with_environment_defaults( manifest_run_defaults.as_ref(), manifest_environment_defaults.as_ref(), - &req, + &manifest, ) { Ok(prepared) => prepared, Err(err) => return ApiError::bad_request(err.to_string()).into_response(), @@ -612,7 +651,9 @@ async fn create_run( return ApiError::bad_request(format!("Run config variable interpolation failed: {err}")) .into_response(); } - let run_id = prepared.run_id.unwrap_or_else(RunId::new); + let run_id = explicit_run_id + .or(prepared.run_id) + .unwrap_or_else(RunId::new); let provider = run_manifest::effective_sandbox_provider(&prepared.settings.run); if let Some(error) = run_manifest::sandbox_provider_policy_error(&state.server_settings(), provider) @@ -653,7 +694,8 @@ async fn create_run( ); create_input.run_id = Some(run_id); create_input.provenance = Some(run_provenance(&headers, &actor)); - create_input.submitted_manifest_bytes = Some(body.to_vec()); + create_input.submitted_manifest_bytes = Some(submitted_manifest_bytes); + create_input.automation = automation; let storage_root = match resolve_interp_string(&state.server_settings().server.storage.root) { Ok(path) => PathBuf::from(path), diff --git a/lib/crates/fabro-server/src/server/tests.rs b/lib/crates/fabro-server/src/server/tests.rs index e53312627..a422fa05b 100644 --- a/lib/crates/fabro-server/src/server/tests.rs +++ b/lib/crates/fabro-server/src/server/tests.rs @@ -9,6 +9,7 @@ use std::sync::{Arc as StdArc, Mutex as StdMutex}; use axum::body::Body; use axum::http::{Method, Request, header}; use chrono::{Duration as ChronoDuration, Utc}; +use fabro_automation::{AutomationId, AutomationTarget}; use fabro_config::ServerSettingsBuilder; use fabro_config::bind::Bind; use fabro_interview::{ @@ -40,6 +41,7 @@ use tracing_subscriber::prelude::*; use tracing_subscriber::{Layer, Registry}; use super::*; +use crate::automation_materializer::AutomationRunMaterializeInput; use crate::github_webhooks::compute_signature; use crate::jwt_auth::{AuthMode, ConfiguredAuth}; use crate::test_support::*; @@ -1662,6 +1664,7 @@ fn slack_app_state_with_secret_sources( http_client: Some(fabro_http::test_http_client().expect("test HTTP client should build")), sandbox_provider_registry: None, shutdown: tokio_util::sync::CancellationToken::new(), + automation_materializer_override: None, }) .expect("slack test app state should build") } @@ -1761,6 +1764,7 @@ fn slack_service_respects_disabled_server_config_even_with_vault_tokens() { http_client: Some(fabro_http::test_http_client().expect("test HTTP client should build")), sandbox_provider_registry: None, shutdown: tokio_util::sync::CancellationToken::new(), + automation_materializer_override: None, }) .expect("slack disabled test app state should build"); @@ -2065,6 +2069,7 @@ methods = ["dev-token"] http_client: Some(fabro_http::test_http_client().expect("test HTTP client should build")), sandbox_provider_registry: None, shutdown: tokio_util::sync::CancellationToken::new(), + automation_materializer_override: None, }) else { panic!("build_app_state should require SESSION_SECRET") }; @@ -2192,6 +2197,7 @@ fn build_test_app_state_with_vault_path(vault_path: &Path) -> anyhow::Result serde_j response_json!(response, StatusCode::CREATED).await } +#[tokio::test] +async fn post_runs_create_regression_keeps_api_behavior_without_automation_metadata() { + let state = TestAppStateBuilder::new().env_lookup(|_| None).build(); + let app = crate::test_support::build_test_router(Arc::clone(&state)); + let mut manifest = minimal_manifest_json(MINIMAL_DOT); + manifest["title"] = json!("API title"); + + let body = post_run_manifest(&app, manifest).await; + let run_id: RunId = body["id"].as_str().unwrap().parse().unwrap(); + + assert_eq!(body["title"], "API title"); + assert!(body["automation"].is_null()); + assert_eq!(body["lifecycle"]["status"]["kind"], "submitted"); + let summary = state + .store + .get_cached_summary(&run_id, Utc::now()) + .await + .unwrap() + .unwrap(); + assert!(summary.automation.is_none()); +} + +#[tokio::test] +async fn create_run_from_manifest_helper_persists_without_automation_metadata() { + let state = TestAppStateBuilder::new().env_lookup(|_| None).build(); + let manifest: RunManifest = serde_json::from_value(minimal_manifest_json(MINIMAL_DOT)).unwrap(); + let submitted_manifest_bytes = serde_json::to_vec(&manifest).unwrap(); + let run_id = RunId::new(); + + let response = Box::pin(handler::runs::create_run_from_manifest( + Arc::clone(&state), + handler::runs::CreateRunFromManifestRequest { + manifest, + submitted_manifest_bytes, + explicit_run_id: Some(run_id), + explicit_title_supplied: false, + actor: Principal::System { + system_kind: SystemActorKind::Engine, + }, + headers: HeaderMap::new(), + automation: None, + }, + )) + .await; + + let body = response_json!(response, StatusCode::CREATED).await; + assert_eq!(body["id"], run_id.to_string()); + assert!(body["automation"].is_null()); + let summary = state + .store + .get_cached_summary(&run_id, Utc::now()) + .await + .unwrap() + .unwrap(); + assert!(summary.automation.is_none()); +} + +#[tokio::test] +async fn create_run_from_manifest_helper_persists_automation_metadata() { + let state = TestAppStateBuilder::new().env_lookup(|_| None).build(); + let manifest: RunManifest = serde_json::from_value(minimal_manifest_json(MINIMAL_DOT)).unwrap(); + let submitted_manifest_bytes = serde_json::to_vec(&manifest).unwrap(); + let run_id = RunId::new(); + let automation = fabro_types::AutomationRef { + id: "nightly".to_string(), + name: Some("Nightly".to_string()), + trigger_id: Some("schedule".to_string()), + }; + + let response = Box::pin(handler::runs::create_run_from_manifest( + Arc::clone(&state), + handler::runs::CreateRunFromManifestRequest { + manifest, + submitted_manifest_bytes, + explicit_run_id: Some(run_id), + explicit_title_supplied: false, + actor: Principal::System { + system_kind: SystemActorKind::Engine, + }, + headers: HeaderMap::new(), + automation: Some(automation.clone()), + }, + )) + .await; + + let body = response_json!(response, StatusCode::CREATED).await; + assert_eq!(body["automation"]["id"], automation.id); + assert_eq!( + body["automation"]["name"], + automation.name.as_deref().unwrap() + ); + assert_eq!( + body["automation"]["trigger_id"], + automation.trigger_id.as_deref().unwrap() + ); + let summary = state + .store + .get_cached_summary(&run_id, Utc::now()) + .await + .unwrap() + .unwrap(); + assert_eq!(summary.automation, Some(automation)); +} + +#[tokio::test] +async fn fake_automation_materializer_injection_captures_input_and_returns_manifest() { + let materialized_manifest: RunManifest = + serde_json::from_value(minimal_manifest_json(MINIMAL_DOT)).unwrap(); + let fake = TestAutomationRunMaterializer::succeed( + materialized_manifest.clone(), + b"{\"fake\":true}".to_vec(), + ); + let state = TestAppStateBuilder::new() + .automation_materializer(fake.clone()) + .build(); + let run_id = RunId::new(); + let user_settings_path = PathBuf::from("/tmp/fabro/settings.toml"); + let temp_root = PathBuf::from("/tmp/fabro/automation"); + let target = AutomationTarget { + repository: "fabro-sh/fabro".to_string(), + ref_selector: "main".to_string(), + workflow: "demo".to_string(), + }; + + let output = state + .materialize_automation_run(AutomationRunMaterializeInput { + automation_id: AutomationId::new("nightly").unwrap(), + target: target.clone(), + run_id, + user_settings_path: user_settings_path.clone(), + temp_root: temp_root.clone(), + }) + .await + .expect("fake materializer should succeed"); + + assert_eq!( + serde_json::to_value(&output.manifest).unwrap(), + serde_json::to_value(&materialized_manifest).unwrap() + ); + assert_eq!(output.submitted_manifest_bytes, b"{\"fake\":true}".to_vec()); + let captured = fake.captured_inputs(); + assert_eq!(captured.len(), 1); + assert_eq!(captured[0].automation_id.as_str(), "nightly"); + assert_eq!(captured[0].target, target); + assert_eq!(captured[0].run_id, run_id); + assert_eq!(captured[0].user_settings_path, user_settings_path); + assert_eq!(captured[0].temp_root, temp_root); +} + async fn mock_openai_title_response<'a>( server: &'a MockServer, title: &str, @@ -5289,6 +5444,7 @@ fn create_github_token_app_state_with_env_lookup_and_llm_catalog_settings( http_client: Some(fabro_http::test_http_client().expect("test HTTP client should build")), sandbox_provider_registry: None, shutdown: tokio_util::sync::CancellationToken::new(), + automation_materializer_override: None, }; let state = build_app_state(config).expect("test app state should build"); if let Some(token) = token { diff --git a/lib/crates/fabro-server/src/test_support.rs b/lib/crates/fabro-server/src/test_support.rs index 54b0972c1..11f80e0e7 100644 --- a/lib/crates/fabro-server/src/test_support.rs +++ b/lib/crates/fabro-server/src/test_support.rs @@ -29,6 +29,8 @@ use tokio_util::sync::CancellationToken; use ulid::Ulid; use crate::auth; +use crate::automation_materializer::AutomationRunMaterializer; +pub use crate::automation_materializer::TestAutomationRunMaterializer; use crate::ip_allowlist::IpAllowlistConfig; use crate::jwt_auth::{AuthMode, ConfiguredAuth}; #[cfg(test)] @@ -71,6 +73,7 @@ pub struct TestAppStateBuilder { server_secret_env: HashMap, env_lookup: EnvLookup, llm_catalog_settings: LlmCatalogSettings, + automation_materializer: Option>, } impl Default for TestAppStateBuilder { @@ -89,6 +92,7 @@ impl Default for TestAppStateBuilder { server_secret_env: HashMap::new(), env_lookup: default_env_lookup(), llm_catalog_settings: LlmCatalogSettings::default(), + automation_materializer: None, } } } @@ -145,6 +149,11 @@ impl TestAppStateBuilder { self } + pub fn automation_materializer(mut self, materializer: TestAutomationRunMaterializer) -> Self { + self.automation_materializer = Some(materializer.into_materializer()); + self + } + pub fn provider_base_url( mut self, provider: impl Into, @@ -240,6 +249,7 @@ impl TestAppStateBuilder { ), sandbox_provider_registry: self.sandbox_provider_registry, shutdown: CancellationToken::new(), + automation_materializer_override: self.automation_materializer, }) } }