From 2d78f9610760f767583298823b2db8880252eee8 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 18:45:59 -0400 Subject: [PATCH] Wire automation store into AppState and expose CRUD REST API (#439) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Loads `AutomationStore` into `AppState` at server startup and exposes five authenticated REST endpoints (`GET/POST /automations`, `GET/PUT/DELETE /automations/{id}`) backed by the existing `fabro-automation` crate. ### Plan Summary - Add `fabro-automation` as a dependency of `fabro-server` and mount `Arc` on `AppState`, computed from a sibling `automations/` directory next to the active config file. - Change `AutomationStore::load` from `async` to synchronous (`std::fs`) so it can run before the Tokio runtime needs to make progress; malformed files now fail startup instead of being silently skipped. - Implement `src/server/handler/automations.rs` with shared helpers for path-ID parsing, `If-Match` (quoted/unquoted) parsing, ETag formatting, and `AutomationStoreError → ApiError` mapping. - HTTP semantics: 201 on create, 404 on missing, 409 on duplicate or stale revision, 422 on domain validation failure, 428 on missing `If-Match`. - Update `TestAppStateBuilder` to derive `active_config_path` from the vault path so each test gets an isolated sibling `automations/` directory; add `try_build()` to allow startup-failure assertions. - Update the OpenAPI spec and generated TypeScript client to include `AutomationListMeta` with a `total` field. ## Key design decisions **Sync load path.** `AutomationStore::load` is now `fn` (not `async fn`), using `std::fs`. A `#[expect(clippy::disallowed_methods)]` annotation explains the rationale: this runs once at startup before the runtime needs to yield, and avoids requiring a Tokio handle at the call site in `build_app_state`. **Fail-fast on malformed files.** Previously, corrupt TOML files were logged as warnings and skipped. Now any parse or validation error during load aborts server startup. The old `warn_load_failure` helper is deleted; tests that relied on skip behaviour are replaced with tests that assert `Err(AutomationStoreError::Parse { .. })` and `Err(AutomationStoreError::InvalidFilename { .. })`. **ETag / If-Match handling.** `parse_required_if_match` strips optional surrounding quotes before parsing the revision, so both `""` and bare `` are accepted from clients. Missing `If-Match` on PUT/DELETE returns **428 Precondition Required**, not 400. **Test isolation.** `TestAppStateBuilder::build` now derives `active_config_path` from `vault_path.with_file_name("settings.toml")` instead of a random temp path, so the sibling `automations/` directory is predictable and cleaned up with the same temp dir. ### Fabro Details
Ran 10 stages in 85m 20s for $33.66 | Stage | Duration | Cost | Retries | |---|---|---|---| | start | 0s | – | 0 | | toolchain | 1s | – | 0 | | preflight_compile | 2m 19s | – | 0 | | preflight_lint | 2m 5s | – | 0 | | fix_lints | 33s | $0.15 | 0 | | implement | 30m 33s | $17.35 | 0 | | simplify_opus | 20m 27s | $11.82 | 0 | | simplify_gpt | 6m 41s | $3.88 | 0 | | verify | 15m 44s | – | 0 | | fixup | 6m 8s | $0.46 | 0 | | **Total** | **85m 20s** | **$33.66** | **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 --- Cargo.lock | 1 + docs/public/api-reference/fabro-api.yaml | 16 + lib/crates/fabro-automation/src/store.rs | 122 ++--- lib/crates/fabro-server/Cargo.toml | 1 + lib/crates/fabro-server/src/server.rs | 22 + .../src/server/handler/automations.rs | 172 ++++++ .../fabro-server/src/server/handler/mod.rs | 2 + lib/crates/fabro-server/src/test_support.rs | 11 +- .../fabro-server/tests/it/api/automations.rs | 501 ++++++++++++++++++ lib/crates/fabro-server/tests/it/api/mod.rs | 1 + .../src/models/automation-list-meta.ts | 25 + .../src/models/automation-list-response.ts | 2 + .../fabro-api-client/src/models/index.ts | 1 + 13 files changed, 799 insertions(+), 78 deletions(-) create mode 100644 lib/crates/fabro-server/src/server/handler/automations.rs create mode 100644 lib/crates/fabro-server/tests/it/api/automations.rs create mode 100644 lib/packages/fabro-api-client/src/models/automation-list-meta.ts diff --git a/Cargo.lock b/Cargo.lock index 51ac4659f..0986b55a6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2352,6 +2352,7 @@ dependencies = [ "fabro-agent", "fabro-api", "fabro-auth", + "fabro-automation", "fabro-build-support", "fabro-client", "fabro-config", diff --git a/docs/public/api-reference/fabro-api.yaml b/docs/public/api-reference/fabro-api.yaml index d28f348f2..eaa272e0c 100644 --- a/docs/public/api-reference/fabro-api.yaml +++ b/docs/public/api-reference/fabro-api.yaml @@ -6011,11 +6011,27 @@ components: additionalProperties: false required: - data + - meta properties: data: type: array items: $ref: "#/components/schemas/Automation" + meta: + $ref: "#/components/schemas/AutomationListMeta" + + AutomationListMeta: + description: Metadata for automation list responses. + type: object + additionalProperties: false + required: + - total + properties: + total: + type: integer + format: int64 + minimum: 0 + description: Total number of configured automation definitions. # ── Pagination ─────────────────────────────────────────────────────── diff --git a/lib/crates/fabro-automation/src/store.rs b/lib/crates/fabro-automation/src/store.rs index 888758e83..6c0541441 100644 --- a/lib/crates/fabro-automation/src/store.rs +++ b/lib/crates/fabro-automation/src/store.rs @@ -6,7 +6,6 @@ use std::time::{SystemTime, UNIX_EPOCH}; use tokio::fs; use tokio::io::AsyncWriteExt as _; use tokio::sync::{Mutex, RwLock}; -use tracing::warn; use crate::{ Automation, AutomationDraft, AutomationId, AutomationReplace, AutomationRevision, @@ -21,9 +20,13 @@ pub struct AutomationStore { } impl AutomationStore { - pub async fn load(dir: impl Into) -> Result { + /// Synchronously load every persisted automation in `dir`. Returns an error + /// if any file fails to parse or validate; the caller decides startup + /// failure policy. Synchronous because it runs once at construction time + /// (typically during server startup) and is invoked from non-async code. + pub fn load(dir: impl Into) -> Result { let dir = dir.into(); - let automations = load_automations(&dir).await?; + let automations = load_automations(&dir)?; Ok(Self { dir, mutations: Mutex::new(()), @@ -118,48 +121,40 @@ impl AutomationStore { } } -async fn load_automations( - dir: &Path, -) -> Result, AutomationStoreError> { - let mut entries = match fs::read_dir(dir).await { +#[expect( + clippy::disallowed_methods, + reason = "Automation directory scan runs once at startup, before the runtime needs to make progress; std::fs avoids needing a Tokio runtime for the caller." +)] +fn load_automations(dir: &Path) -> Result, AutomationStoreError> { + let entries = match std::fs::read_dir(dir) { Ok(entries) => entries, - Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(HashMap::new()), + Err(err) if err.kind() == ErrorKind::NotFound => return Ok(HashMap::new()), Err(err) => return Err(AutomationStoreError::io(dir, err)), }; let mut automations = HashMap::new(); - while let Some(entry) = entries - .next_entry() - .await - .map_err(|err| AutomationStoreError::io(dir, err))? - { + for entry in entries { + let entry = entry.map_err(|err| AutomationStoreError::io(dir, err))?; let path = entry.path(); - let file_type = match entry.file_type().await { - Ok(file_type) => file_type, - Err(err) => { - warn_load_failure(&path, &AutomationStoreError::io(path.clone(), err)); - continue; - } - }; + let file_type = entry + .file_type() + .map_err(|err| AutomationStoreError::io(&path, err))?; if !file_type.is_file() || !is_toml_file(&path) { continue; } - - match load_automation_file(&path).await { - Ok(automation) => { - automations.insert(automation.id.clone(), automation); - } - Err(err) => warn_load_failure(&path, &err), - } + let automation = load_automation_file(&path)?; + automations.insert(automation.id.clone(), automation); } Ok(automations) } -async fn load_automation_file(path: &Path) -> Result { +#[expect( + clippy::disallowed_methods, + reason = "Sync sibling of `load_automations`; only invoked from the synchronous startup load path." +)] +fn load_automation_file(path: &Path) -> Result { let id = id_from_path(path)?; - let bytes = fs::read(path) - .await - .map_err(|err| AutomationStoreError::io(path, err))?; + let bytes = std::fs::read(path).map_err(|err| AutomationStoreError::io(path, err))?; Automation::from_persisted_path(id, &bytes, path) } @@ -272,15 +267,6 @@ fn automation_path(dir: &Path, id: &AutomationId) -> PathBuf { dir.join(format!("{id}.toml")) } -fn warn_load_failure(path: &Path, err: &AutomationStoreError) { - warn!( - path = %path.display(), - failure_kind = err.kind(), - error = %err, - "Skipping automation file" - ); -} - #[cfg(test)] mod tests { use tokio::fs; @@ -336,39 +322,19 @@ mod tests { #[tokio::test] async fn missing_directory_loads_empty_store() { let dir = tempfile::tempdir().unwrap(); - let store = AutomationStore::load(dir.path().join("automations")) - .await - .unwrap(); + let store = AutomationStore::load(dir.path().join("automations")).unwrap(); assert!(store.list().await.is_empty()); } #[tokio::test] - async fn load_skips_invalid_files_and_keeps_valid_automations() { + async fn load_ignores_non_toml_files_and_keeps_valid_automations() { let dir = tempfile::tempdir().unwrap(); let automation_dir = dir.path().join("automations"); fs::create_dir_all(&automation_dir).await.unwrap(); fs::write(automation_dir.join("notes.txt"), "ignore") .await .unwrap(); - fs::write(automation_dir.join("bad name.toml"), "name = \"Bad\"") - .await - .unwrap(); - fs::write(automation_dir.join("broken.toml"), "not valid toml =") - .await - .unwrap(); - fs::write( - automation_dir.join("empty-name.toml"), - r#" -name = " " - -[target] -repository = "fabro-sh/fabro" -workflow = "release" -"#, - ) - .await - .unwrap(); fs::write( automation_dir.join("valid.toml"), r#" @@ -383,7 +349,7 @@ workflow = "release" .await .unwrap(); - let store = AutomationStore::load(&automation_dir).await.unwrap(); + let store = AutomationStore::load(&automation_dir).unwrap(); let automations = store.list().await; assert_eq!(automations.len(), 1); @@ -392,28 +358,36 @@ workflow = "release" } #[tokio::test] - async fn create_does_not_overwrite_existing_malformed_file() { + async fn load_fails_on_malformed_toml() { let dir = tempfile::tempdir().unwrap(); let automation_dir = dir.path().join("automations"); fs::create_dir_all(&automation_dir).await.unwrap(); - let path = automation_dir.join("nightly.toml"); - fs::write(&path, "not valid toml =").await.unwrap(); + fs::write(automation_dir.join("broken.toml"), "not valid toml =") + .await + .unwrap(); - let store = AutomationStore::load(&automation_dir).await.unwrap(); - let result = store.create(draft("nightly", "Nightly")).await; + let err = AutomationStore::load(&automation_dir).unwrap_err(); + assert!(matches!(err, AutomationStoreError::Parse { .. })); + } - assert!(matches!( - result, - Err(AutomationStoreError::AlreadyExists { id }) if id.as_str() == "nightly" - )); - assert_eq!(fs::read_to_string(&path).await.unwrap(), "not valid toml ="); + #[tokio::test] + async fn load_fails_on_invalid_filename_id() { + let dir = tempfile::tempdir().unwrap(); + let automation_dir = dir.path().join("automations"); + fs::create_dir_all(&automation_dir).await.unwrap(); + fs::write(automation_dir.join("Bad Name.toml"), "name = \"Bad\"") + .await + .unwrap(); + + let err = AutomationStore::load(&automation_dir).unwrap_err(); + assert!(matches!(err, AutomationStoreError::InvalidFilename { .. })); } #[tokio::test] async fn create_replace_and_delete_round_trip_files_and_revisions() { let dir = tempfile::tempdir().unwrap(); let automation_dir = dir.path().join("automations"); - let store = AutomationStore::load(&automation_dir).await.unwrap(); + let store = AutomationStore::load(&automation_dir).unwrap(); let created = store.create(draft("nightly", "Nightly")).await.unwrap(); let path = automation_dir.join("nightly.toml"); diff --git a/lib/crates/fabro-server/Cargo.toml b/lib/crates/fabro-server/Cargo.toml index 6aebf6cb0..89611b971 100644 --- a/lib/crates/fabro-server/Cargo.toml +++ b/lib/crates/fabro-server/Cargo.toml @@ -21,6 +21,7 @@ required-features = ["test-support"] workspace = true [dependencies] +fabro-automation = { path = "../fabro-automation" } fabro-auth = { path = "../fabro-auth" } fabro-install = { path = "../fabro-install" } fabro-spa = { path = "../fabro-spa" } diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index 43721bec3..44357b2a4 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -50,6 +50,7 @@ pub use fabro_api::types::{ WriteBlobResponse, }; use fabro_auth::{CredentialSource, VaultCredentialSource, auth_issue_message}; +use fabro_automation::AutomationStore; #[cfg(test)] use fabro_config::RunSettingsBuilder; use fabro_config::daemon::ServerDaemon; @@ -1007,6 +1008,7 @@ pub struct AppState { store: Arc, session_runtimes: SessionRuntimeManager, artifact_store: ArtifactStore, + automation_store: Arc, worker_tokens: WorkerTokenKeys, started_at: Instant, resource_sampler: resource_sampler::ResourceSampler, @@ -1043,6 +1045,12 @@ pub struct AppState { type PullRequestCreateLocks = Arc>>>>; +impl AppState { + pub(crate) fn automation_store(&self) -> &AutomationStore { + &self.automation_store + } +} + pub(crate) struct AskFabroReadiness { default_model: Option, } @@ -2163,6 +2171,13 @@ fn build_sandbox_provider_registry( SandboxProviderRegistry::new(providers) } +fn automation_dir_for_active_config(active_config_path: &std::path::Path) -> PathBuf { + active_config_path + .parent() + .unwrap_or_else(|| std::path::Path::new(".")) + .join("automations") +} + pub(crate) fn build_app_state(config: AppStateConfig) -> anyhow::Result> { let AppStateConfig { resolved_settings, @@ -2182,6 +2197,12 @@ pub(crate) fn build_app_state(config: AppStateConfig) -> anyhow::Result anyhow::Result, + meta: AutomationListMeta, +} + +#[derive(Serialize)] +struct AutomationListMeta { + total: usize, +} + +pub(super) fn routes() -> Router> { + Router::new() + .route( + "/automations", + get(list_automations).post(create_automation), + ) + .route( + "/automations/{id}", + get(get_automation) + .put(replace_automation) + .delete(delete_automation), + ) +} + +async fn list_automations(_auth: RequiredUser, State(state): State>) -> Response { + let data = state.automation_store().list().await; + let total = data.len(); + ( + StatusCode::OK, + Json(AutomationListResponse { + data, + meta: AutomationListMeta { total }, + }), + ) + .into_response() +} + +async fn create_automation( + _auth: RequiredUser, + State(state): State>, + Json(draft): Json, +) -> Result { + let automation = state.automation_store().create(draft).await?; + Ok((StatusCode::CREATED, Json(automation)).into_response()) +} + +async fn get_automation( + _auth: RequiredUser, + State(state): State>, + Path(id): Path, +) -> Result { + let id = parse_path_id(id)?; + match state.automation_store().get(&id).await { + Some(automation) => Ok(automation_with_etag_response(StatusCode::OK, automation)), + None => Err(ApiError::not_found(format!("automation not found: {id}"))), + } +} + +async fn replace_automation( + _auth: RequiredUser, + State(state): State>, + headers: HeaderMap, + Path(id): Path, + Json(replacement): Json, +) -> Result { + let id = parse_path_id(id)?; + let expected = parse_required_if_match(&headers, &id)?; + let automation = state + .automation_store() + .replace(&id, &expected, replacement) + .await?; + Ok(automation_with_etag_response(StatusCode::OK, automation)) +} + +async fn delete_automation( + _auth: RequiredUser, + State(state): State>, + headers: HeaderMap, + Path(id): Path, +) -> Result { + let id = parse_path_id(id)?; + let expected = parse_required_if_match(&headers, &id)?; + state.automation_store().delete(&id, &expected).await?; + Ok(StatusCode::NO_CONTENT.into_response()) +} + +fn parse_path_id(id: String) -> Result { + AutomationId::new(id) + .map_err(|err| ApiError::bad_request(format!("invalid automation id: {err}"))) +} + +fn parse_required_if_match( + headers: &HeaderMap, + id: &AutomationId, +) -> Result { + let Some(value) = headers.get(header::IF_MATCH) else { + return Err(ApiError::new( + StatusCode::PRECONDITION_REQUIRED, + format!("If-Match header is required for automation: {id}"), + )); + }; + let value = value + .to_str() + .map_err(|_| ApiError::bad_request("If-Match header must be visible ASCII"))?; + let value = unquote_etag(value.trim()); + value.parse::().map_err(|err| { + ApiError::bad_request(format!("invalid If-Match automation revision: {err}")) + }) +} + +fn unquote_etag(value: &str) -> &str { + value + .strip_prefix('"') + .and_then(|unquoted| unquoted.strip_suffix('"')) + .unwrap_or(value) +} + +fn automation_with_etag_response(status: StatusCode, automation: Automation) -> Response { + let etag = HeaderValue::from_str(&format!("\"{}\"", automation.revision)) + .expect("automation revisions are valid ETag header values"); + let mut response = (status, Json(automation)).into_response(); + response.headers_mut().insert(header::ETAG, etag); + response +} + +impl From for ApiError { + fn from(err: AutomationStoreError) -> Self { + match err { + AutomationStoreError::NotFound { id } => { + Self::not_found(format!("automation not found: {id}")) + } + AutomationStoreError::AlreadyExists { id } => Self::new( + StatusCode::CONFLICT, + format!("automation already exists: {id}"), + ), + AutomationStoreError::StaleRevision { id, .. } => Self::new( + StatusCode::CONFLICT, + format!("automation revision is stale: {id}"), + ), + AutomationStoreError::Validation { source } => { + Self::new(StatusCode::UNPROCESSABLE_ENTITY, source.to_string()) + } + // The handlers parse `If-Match` before reaching the store, so a + // missing-revision error from the store would indicate an internal + // bug rather than a client problem. + AutomationStoreError::MissingRevision { .. } + | AutomationStoreError::InvalidFilename { .. } + | AutomationStoreError::Parse { .. } + | AutomationStoreError::InvalidUtf8 { .. } + | AutomationStoreError::Serialize { .. } + | AutomationStoreError::Io { .. } => Self::new( + StatusCode::INTERNAL_SERVER_ERROR, + "automation store operation failed", + ), + } + } +} diff --git a/lib/crates/fabro-server/src/server/handler/mod.rs b/lib/crates/fabro-server/src/server/handler/mod.rs index 1bae48352..0804cccb3 100644 --- a/lib/crates/fabro-server/src/server/handler/mod.rs +++ b/lib/crates/fabro-server/src/server/handler/mod.rs @@ -6,6 +6,7 @@ use axum::routing::{get, post}; use super::{ApiError, AppState, IntoResponse, Response, StatusCode, demo}; mod artifacts; +mod automations; mod billing; mod completions; pub(in crate::server) mod events; @@ -155,6 +156,7 @@ pub(super) fn real_routes() -> Router> { .merge(billing::routes()) .merge(pull_requests::routes()) .merge(artifacts::routes()) + .merge(automations::routes()) .merge(sandbox::routes()) .merge(sandboxes::routes()) .merge(lifecycle::routes()) diff --git a/lib/crates/fabro-server/src/test_support.rs b/lib/crates/fabro-server/src/test_support.rs index 08edbbdca..54b0972c1 100644 --- a/lib/crates/fabro-server/src/test_support.rs +++ b/lib/crates/fabro-server/src/test_support.rs @@ -198,6 +198,10 @@ impl TestAppStateBuilder { } pub fn build(self) -> Arc { + self.try_build().expect("test app state should build") + } + + pub fn try_build(self) -> anyhow::Result> { let (store, artifact_store) = self.store_bundle.unwrap_or_else(test_store_bundle); let vault_path = self.vault_path.unwrap_or_else(test_secret_store_path); if !self.vault_entries.is_empty() { @@ -211,9 +215,9 @@ impl TestAppStateBuilder { let server_env_path = self .server_env_path .unwrap_or_else(|| vault_path.with_file_name("server.env")); - let active_config_path = self.active_config_path.unwrap_or_else(|| { - std::env::temp_dir().join(format!("fabro-test-settings-{}.toml", Ulid::new())) - }); + let active_config_path = self + .active_config_path + .unwrap_or_else(|| vault_path.with_file_name("settings.toml")); build_app_state(AppStateConfig { resolved_settings: resolved_runtime_settings_for_tests( self.server_settings, @@ -237,7 +241,6 @@ impl TestAppStateBuilder { sandbox_provider_registry: self.sandbox_provider_registry, shutdown: CancellationToken::new(), }) - .expect("test app state should build") } } diff --git a/lib/crates/fabro-server/tests/it/api/automations.rs b/lib/crates/fabro-server/tests/it/api/automations.rs new file mode 100644 index 000000000..37189c750 --- /dev/null +++ b/lib/crates/fabro-server/tests/it/api/automations.rs @@ -0,0 +1,501 @@ +use std::path::PathBuf; + +use axum::body::Body; +use axum::http::{Method, Request, StatusCode, header}; +use fabro_server::server::build_router; +use fabro_server::test_support::{TestAppStateBuilder, build_test_router, test_auth_mode}; +use serde_json::{Value, json}; +use tower::ServiceExt; + +use crate::helpers::{api, checked_response, response_json, response_status}; + +fn automation_body(id: &str, name: &str) -> Value { + json!({ + "id": id, + "name": name, + "description": "Runs on a schedule.", + "enabled": true, + "target": { + "repository": "fabro-sh/fabro", + "ref": "main", + "workflow": "release" + }, + "triggers": [ + { + "type": "api", + "id": "manual", + "enabled": true + }, + { + "type": "schedule", + "id": "nightly", + "enabled": true, + "expression": "0 3 * * *" + } + ] + }) +} + +fn replacement_body(name: &str) -> Value { + json!({ + "name": name, + "description": null, + "enabled": false, + "target": { + "repository": "fabro-sh/fabro", + "ref": "main", + "workflow": "release" + }, + "triggers": [ + { + "type": "api", + "id": "manual", + "enabled": false + } + ] + }) +} + +fn automation_app() -> (axum::Router, tempfile::TempDir, PathBuf) { + let temp_dir = tempfile::tempdir().expect("automation test tempdir should be created"); + let active_config_path = temp_dir.path().join("settings.toml"); + let automation_dir = temp_dir.path().join("automations"); + let state = TestAppStateBuilder::new() + .active_config_path(active_config_path) + .build(); + (build_test_router(state), temp_dir, automation_dir) +} + +fn json_request(method: Method, path: &str, body: &Value) -> Request { + Request::builder() + .method(method) + .uri(api(path)) + .header(header::CONTENT_TYPE, "application/json") + .body(Body::from( + serde_json::to_vec(&body).expect("automation fixture should serialize"), + )) + .expect("automation JSON request should build") +} + +fn empty_request(method: Method, path: &str) -> Request { + Request::builder() + .method(method) + .uri(api(path)) + .body(Body::empty()) + .expect("automation request should build") +} + +fn request_with_if_match( + method: Method, + path: &str, + revision: &str, + body: Option, +) -> Request { + let mut builder = Request::builder() + .method(method) + .uri(api(path)) + .header(header::IF_MATCH, revision); + let body = match body { + Some(value) => { + builder = builder.header(header::CONTENT_TYPE, "application/json"); + Body::from(serde_json::to_vec(&value).expect("automation fixture should serialize")) + } + None => Body::empty(), + }; + builder + .body(body) + .expect("automation If-Match request should build") +} + +async fn create_automation(app: &axum::Router, id: &str, name: &str) -> Value { + let response = app + .clone() + .oneshot(json_request( + Method::POST, + "/automations", + &automation_body(id, name), + )) + .await + .expect("create automation should respond"); + response_json(response, StatusCode::CREATED, "POST /api/v1/automations").await +} + +fn revision_from(body: &Value) -> &str { + body["revision"] + .as_str() + .expect("automation response should include a revision") +} + +#[tokio::test] +async fn empty_automation_list_returns_total_zero() { + let (app, _temp_dir, _automation_dir) = automation_app(); + + let response = app + .oneshot(empty_request(Method::GET, "/automations")) + .await + .expect("list automations should respond"); + let body = response_json(response, StatusCode::OK, "GET /api/v1/automations").await; + + assert_eq!( + body, + json!({ + "data": [], + "meta": { + "total": 0 + } + }) + ); +} + +#[tokio::test] +async fn create_automation_persists_sibling_toml_file() { + let (app, _temp_dir, automation_dir) = automation_app(); + + let body = create_automation(&app, "nightly", "Nightly").await; + + assert_eq!(body["id"], "nightly"); + assert_eq!(body["name"], "Nightly"); + assert!(automation_dir.join("nightly.toml").exists()); +} + +#[tokio::test] +async fn list_automations_returns_items_sorted_by_id() { + let (app, _temp_dir, _automation_dir) = automation_app(); + create_automation(&app, "zulu", "Zulu").await; + create_automation(&app, "alpha", "Alpha").await; + + let response = app + .oneshot(empty_request(Method::GET, "/automations")) + .await + .expect("list automations should respond"); + let body = response_json(response, StatusCode::OK, "GET /api/v1/automations").await; + + assert_eq!(body["meta"]["total"], 2); + assert_eq!(body["data"][0]["id"], "alpha"); + assert_eq!(body["data"][1]["id"], "zulu"); +} + +#[tokio::test] +async fn duplicate_automation_create_returns_conflict() { + let (app, _temp_dir, _automation_dir) = automation_app(); + create_automation(&app, "nightly", "Nightly").await; + + let response = app + .oneshot(json_request( + Method::POST, + "/automations", + &automation_body("nightly", "Duplicate"), + )) + .await + .expect("duplicate create should respond"); + + response_status( + response, + StatusCode::CONFLICT, + "POST /api/v1/automations duplicate", + ) + .await; +} + +#[tokio::test] +async fn get_automation_returns_current_etag() { + let (app, _temp_dir, _automation_dir) = automation_app(); + let created = create_automation(&app, "nightly", "Nightly").await; + let revision = revision_from(&created); + + let response = app + .oneshot(empty_request(Method::GET, "/automations/nightly")) + .await + .expect("get automation should respond"); + let response = + checked_response(response, StatusCode::OK, "GET /api/v1/automations/nightly").await; + + assert_eq!( + response + .headers() + .get(header::ETAG) + .expect("GET automation should include ETag"), + &format!("\"{revision}\"") + ); + let body = crate::helpers::body_json(response.into_body()).await; + assert_eq!(body["revision"], revision); +} + +#[tokio::test] +async fn replace_automation_accepts_unquoted_if_match_and_returns_new_etag() { + let (app, _temp_dir, _automation_dir) = automation_app(); + let created = create_automation(&app, "nightly", "Nightly").await; + let revision = revision_from(&created); + + let response = app + .oneshot(request_with_if_match( + Method::PUT, + "/automations/nightly", + revision, + Some(replacement_body("Updated")), + )) + .await + .expect("replace automation should respond"); + let response = + checked_response(response, StatusCode::OK, "PUT /api/v1/automations/nightly").await; + let etag = response + .headers() + .get(header::ETAG) + .expect("PUT automation should include ETag") + .to_str() + .expect("ETag should be ASCII") + .to_string(); + let body = crate::helpers::body_json(response.into_body()).await; + + assert_eq!(body["name"], "Updated"); + assert_ne!(body["revision"], revision); + assert_eq!(etag, format!("\"{}\"", revision_from(&body))); +} + +#[tokio::test] +async fn stale_automation_replace_returns_conflict() { + let (app, _temp_dir, _automation_dir) = automation_app(); + let created = create_automation(&app, "nightly", "Nightly").await; + let stale_revision = revision_from(&created).to_string(); + + let replaced = app + .clone() + .oneshot(request_with_if_match( + Method::PUT, + "/automations/nightly", + &stale_revision, + Some(replacement_body("Updated")), + )) + .await + .expect("first replace should respond"); + response_status( + replaced, + StatusCode::OK, + "PUT /api/v1/automations/nightly first replace", + ) + .await; + + let response = app + .oneshot(request_with_if_match( + Method::PUT, + "/automations/nightly", + &stale_revision, + Some(replacement_body("Stale")), + )) + .await + .expect("stale replace should respond"); + + response_status( + response, + StatusCode::CONFLICT, + "PUT /api/v1/automations/nightly stale", + ) + .await; +} + +#[tokio::test] +async fn replace_and_delete_automation_require_if_match() { + let (app, _temp_dir, _automation_dir) = automation_app(); + create_automation(&app, "nightly", "Nightly").await; + + let replace_response = app + .clone() + .oneshot(json_request( + Method::PUT, + "/automations/nightly", + &replacement_body("Updated"), + )) + .await + .expect("replace without If-Match should respond"); + response_status( + replace_response, + StatusCode::PRECONDITION_REQUIRED, + "PUT /api/v1/automations/nightly without If-Match", + ) + .await; + + let delete_response = app + .oneshot(empty_request(Method::DELETE, "/automations/nightly")) + .await + .expect("delete without If-Match should respond"); + response_status( + delete_response, + StatusCode::PRECONDITION_REQUIRED, + "DELETE /api/v1/automations/nightly without If-Match", + ) + .await; +} + +#[tokio::test] +async fn delete_automation_removes_file_and_resource() { + let (app, _temp_dir, automation_dir) = automation_app(); + let created = create_automation(&app, "nightly", "Nightly").await; + let revision = revision_from(&created); + + let response = app + .clone() + .oneshot(request_with_if_match( + Method::DELETE, + "/automations/nightly", + &format!("\"{revision}\""), + None, + )) + .await + .expect("delete automation should respond"); + response_status( + response, + StatusCode::NO_CONTENT, + "DELETE /api/v1/automations/nightly", + ) + .await; + + assert!(!automation_dir.join("nightly.toml").exists()); + let response = app + .oneshot(empty_request(Method::GET, "/automations/nightly")) + .await + .expect("get deleted automation should respond"); + response_status( + response, + StatusCode::NOT_FOUND, + "GET /api/v1/automations/nightly after delete", + ) + .await; +} + +#[tokio::test] +async fn invalid_trigger_ids_are_unprocessable() { + let (app, _temp_dir, _automation_dir) = automation_app(); + let mut body = automation_body("nightly", "Nightly"); + body["triggers"][0]["id"] = json!("Bad!"); + + let response = app + .oneshot(json_request(Method::POST, "/automations", &body)) + .await + .expect("invalid trigger id create should respond"); + + response_status( + response, + StatusCode::UNPROCESSABLE_ENTITY, + "POST /api/v1/automations invalid trigger id", + ) + .await; +} + +#[tokio::test] +async fn empty_automation_name_is_unprocessable() { + let (app, _temp_dir, _automation_dir) = automation_app(); + let mut body = automation_body("nightly", "Nightly"); + body["name"] = json!(" "); + + let response = app + .oneshot(json_request(Method::POST, "/automations", &body)) + .await + .expect("empty automation name create should respond"); + + response_status( + response, + StatusCode::UNPROCESSABLE_ENTITY, + "POST /api/v1/automations empty name", + ) + .await; +} + +#[tokio::test] +async fn duplicate_trigger_ids_are_unprocessable() { + let (app, _temp_dir, _automation_dir) = automation_app(); + let mut body = automation_body("nightly", "Nightly"); + body["triggers"][1]["id"] = json!("manual"); + + let response = app + .oneshot(json_request(Method::POST, "/automations", &body)) + .await + .expect("duplicate trigger create should respond"); + + response_status( + response, + StatusCode::UNPROCESSABLE_ENTITY, + "POST /api/v1/automations duplicate trigger ids", + ) + .await; +} + +#[tokio::test] +async fn second_api_trigger_is_unprocessable() { + let (app, _temp_dir, _automation_dir) = automation_app(); + let mut body = automation_body("nightly", "Nightly"); + body["triggers"][1] = json!({ + "type": "api", + "id": "manual2", + "enabled": true + }); + + let response = app + .oneshot(json_request(Method::POST, "/automations", &body)) + .await + .expect("second API trigger create should respond"); + + response_status( + response, + StatusCode::UNPROCESSABLE_ENTITY, + "POST /api/v1/automations second API trigger", + ) + .await; +} + +#[tokio::test] +async fn invalid_schedule_expression_is_unprocessable() { + let (app, _temp_dir, _automation_dir) = automation_app(); + let mut body = automation_body("nightly", "Nightly"); + body["triggers"][1]["expression"] = json!("60 3 * * *"); + + let response = app + .oneshot(json_request(Method::POST, "/automations", &body)) + .await + .expect("invalid schedule create should respond"); + + response_status( + response, + StatusCode::UNPROCESSABLE_ENTITY, + "POST /api/v1/automations invalid schedule", + ) + .await; +} + +#[tokio::test] +async fn automation_store_malformed_persisted_toml_fails_startup() { + let temp_dir = tempfile::tempdir().expect("automation test tempdir should be created"); + let automation_dir = temp_dir.path().join("automations"); + tokio::fs::create_dir_all(&automation_dir) + .await + .expect("automation dir should be created"); + tokio::fs::write(automation_dir.join("broken.toml"), "not valid toml =") + .await + .expect("broken automation fixture should be written"); + + let result = TestAppStateBuilder::new() + .active_config_path(temp_dir.path().join("settings.toml")) + .try_build(); + + assert!(result.is_err()); +} + +#[tokio::test] +async fn automations_routes_require_authenticated_user() { + let temp_dir = tempfile::tempdir().expect("automation test tempdir should be created"); + let state = TestAppStateBuilder::new() + .active_config_path(temp_dir.path().join("settings.toml")) + .build(); + let app = build_router(state, test_auth_mode()); + + let response = app + .oneshot(empty_request(Method::GET, "/automations")) + .await + .expect("unauthenticated automation list should respond"); + + response_status( + response, + StatusCode::UNAUTHORIZED, + "GET /api/v1/automations without auth", + ) + .await; +} diff --git a/lib/crates/fabro-server/tests/it/api/mod.rs b/lib/crates/fabro-server/tests/it/api/mod.rs index 808d71b6f..843501072 100644 --- a/lib/crates/fabro-server/tests/it/api/mod.rs +++ b/lib/crates/fabro-server/tests/it/api/mod.rs @@ -1,4 +1,5 @@ mod auth_sessions; +mod automations; mod cli_auth_token; mod docs; mod events; diff --git a/lib/packages/fabro-api-client/src/models/automation-list-meta.ts b/lib/packages/fabro-api-client/src/models/automation-list-meta.ts new file mode 100644 index 000000000..12524dba9 --- /dev/null +++ b/lib/packages/fabro-api-client/src/models/automation-list-meta.ts @@ -0,0 +1,25 @@ +/* tslint:disable */ +/* eslint-disable */ +/** + * Fabro Run API + * HTTP API for managing Fabro workflow run executions. + * + * The version of the OpenAPI document: 0.1.0 + * + * + * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). + * https://openapi-generator.tech + * Do not edit the class manually. + */ + + + +/** + * Metadata for automation list responses. + */ +export interface AutomationListMeta { + /** + * Total number of configured automation definitions. + */ + 'total': number; +} diff --git a/lib/packages/fabro-api-client/src/models/automation-list-response.ts b/lib/packages/fabro-api-client/src/models/automation-list-response.ts index d3b89ab51..e100bb637 100644 --- a/lib/packages/fabro-api-client/src/models/automation-list-response.ts +++ b/lib/packages/fabro-api-client/src/models/automation-list-response.ts @@ -16,10 +16,12 @@ // May contain unused imports in some cases // @ts-ignore import type { Automation } from './automation'; +import type { AutomationListMeta } from './automation-list-meta'; /** * List envelope for automation definitions. */ export interface AutomationListResponse { 'data': Array; + 'meta': AutomationListMeta; } diff --git a/lib/packages/fabro-api-client/src/models/index.ts b/lib/packages/fabro-api-client/src/models/index.ts index 976cadc60..e7a9dc111 100644 --- a/lib/packages/fabro-api-client/src/models/index.ts +++ b/lib/packages/fabro-api-client/src/models/index.ts @@ -31,6 +31,7 @@ export * from './auth-session-user'; export * from './auth-sessions-response'; export * from './automation'; export * from './automation-api-trigger'; +export * from './automation-list-meta'; export * from './automation-list-response'; export * from './automation-ref'; export * from './automation-schedule-trigger';