checkpoint

⚒️ Generated with [Fabro](https://fabro.sh)
This commit is contained in:
Fabro 2026-05-27 20:54:07 -04:00
parent fb63fb1d37
commit a1ab7f6ae6
6 changed files with 1192 additions and 18 deletions

498
run.json

File diff suppressed because one or more lines are too long

View file

@ -0,0 +1,491 @@
diff --git a/lib/crates/fabro-server/src/principal_middleware.rs b/lib/crates/fabro-server/src/principal_middleware.rs
index acc9eb250..912f36d43 100644
--- a/lib/crates/fabro-server/src/principal_middleware.rs
+++ b/lib/crates/fabro-server/src/principal_middleware.rs
@@ -57,6 +57,7 @@ pub(crate) struct RequestAuth(pub(crate) AuthContextSlot);
pub(crate) struct RequiredUser(pub(crate) UserPrincipal);
pub(crate) struct RequiredRunManagementActor(pub(crate) Principal);
+pub(crate) struct RequiredRunToolActor(pub(crate) Principal);
pub(crate) struct RequireRunScoped(pub(crate) RunId);
pub(crate) struct RequireRunManagementTarget(pub(crate) RunId, pub(crate) Principal);
pub(crate) struct RequireRunBlob(pub(crate) RunId, pub(crate) RunBlobId);
@@ -228,6 +229,19 @@ impl<S: Send + Sync> FromRequestParts<S> for RequiredRunManagementActor {
}
}
+impl<S: Send + Sync> FromRequestParts<S> for RequiredRunToolActor {
+ type Rejection = ApiError;
+
+ async fn from_request_parts(parts: &mut Parts, _: &S) -> Result<Self, Self::Rejection> {
+ let slot = parts
+ .extensions
+ .get::<AuthContextSlot>()
+ .cloned()
+ .unwrap_or_else(AuthContextSlot::initial);
+ require_run_tool_actor(&slot).map(Self)
+ }
+}
+
impl FromRequestParts<Arc<AppState>> for RequireRunScoped {
type Rejection = Response;
@@ -405,6 +419,10 @@ pub(crate) fn require_run_management_actor(slot: &AuthContextSlot) -> Result<Pri
}
}
+pub(crate) fn require_run_tool_actor(slot: &AuthContextSlot) -> Result<Principal, ApiError> {
+ require_run_management_actor(slot)
+}
+
fn require_worker_or_user_for_run(
slot: &AuthContextSlot,
route_run_id: &RunId,
diff --git a/lib/crates/fabro-server/src/server/handler/automations.rs b/lib/crates/fabro-server/src/server/handler/automations.rs
index 0c3ea067a..94b873fc4 100644
--- a/lib/crates/fabro-server/src/server/handler/automations.rs
+++ b/lib/crates/fabro-server/src/server/handler/automations.rs
@@ -1,16 +1,26 @@
use std::sync::Arc;
use axum::http::{HeaderMap, HeaderValue, header};
+use axum_extra::extract::Query as ExtraQuery;
use fabro_automation::{
- Automation, AutomationDraft, AutomationId, AutomationReplace, AutomationRevision,
- AutomationStoreError,
+ ApiTrigger, Automation, AutomationDraft, AutomationId, AutomationReplace, AutomationRevision,
+ AutomationStoreError, AutomationTrigger,
};
+use fabro_config::Storage;
+use fabro_types::{AutomationRef, RunId};
use serde::Serialize;
use super::super::{
- ApiError, AppState, IntoResponse, Json, Path, RequiredUser, Response, Router, State,
- StatusCode, get,
+ ApiError, AppState, IntoResponse, Json, PaginationParams, Path, RequiredUser, Response, Router,
+ State, StatusCode, get, paginate_items,
};
+use super::runs;
+use crate::automation_materializer::{
+ AutomationRunMaterializeError, AutomationRunMaterializeInput,
+};
+use crate::principal_middleware::RequiredRunToolActor;
+
+const AUTOMATION_API_TRIGGER_DISABLED_CODE: &str = "automation_api_trigger_disabled";
#[derive(Serialize)]
struct AutomationListResponse {
@@ -29,6 +39,10 @@ pub(super) fn routes() -> Router<Arc<AppState>> {
"/automations",
get(list_automations).post(create_automation),
)
+ .route(
+ "/automations/{id}/runs",
+ get(list_automation_runs).post(create_automation_run),
+ )
.route(
"/automations/{id}",
get(get_automation)
@@ -50,6 +64,116 @@ async fn list_automations(_auth: RequiredUser, State(state): State<Arc<AppState>
.into_response()
}
+async fn list_automation_runs(
+ _auth: RequiredUser,
+ State(state): State<Arc<AppState>>,
+ Path(id): Path<String>,
+ ExtraQuery(pagination): ExtraQuery<PaginationParams>,
+) -> Response {
+ let id = match parse_path_id(id) {
+ Ok(id) => id,
+ Err(err) => return err.into_response(),
+ };
+ if state.automation_store().get(&id).await.is_none() {
+ return ApiError::not_found(format!("automation not found: {id}")).into_response();
+ }
+
+ let entries = match state
+ .store
+ .list_cached_runs(&fabro_store::ListRunsQuery::default(), chrono::Utc::now())
+ .await
+ {
+ Ok(entries) => entries,
+ Err(err) => {
+ return ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string())
+ .into_response();
+ }
+ };
+
+ let mut runs: Vec<fabro_types::Run> = entries
+ .into_iter()
+ .map(|entry| entry.summary)
+ .filter(|run| {
+ run.automation
+ .as_ref()
+ .is_some_and(|automation| automation.id == id.as_str())
+ })
+ .collect();
+ runs.sort_by(|a, b| {
+ b.timestamps
+ .created_at
+ .cmp(&a.timestamps.created_at)
+ .then_with(|| b.id.cmp(&a.id))
+ });
+
+ let total = runs.len() as u64;
+ let decorated = state.decorate_run_summaries(runs).await;
+ let (data, has_more) = paginate_items(decorated, &pagination);
+
+ (
+ StatusCode::OK,
+ Json(serde_json::json!({
+ "data": data,
+ "meta": { "has_more": has_more, "total": total }
+ })),
+ )
+ .into_response()
+}
+
+async fn create_automation_run(
+ RequiredRunToolActor(actor): RequiredRunToolActor,
+ State(state): State<Arc<AppState>>,
+ headers: HeaderMap,
+ Path(id): Path<String>,
+) -> Response {
+ let id = match parse_path_id(id) {
+ Ok(id) => id,
+ Err(err) => return err.into_response(),
+ };
+ let Some(automation) = state.automation_store().get(&id).await else {
+ return ApiError::not_found(format!("automation not found: {id}")).into_response();
+ };
+ let Some(api_trigger) = enabled_api_trigger(&automation) else {
+ return automation_api_trigger_disabled_error().into_response();
+ };
+ let api_trigger_id = api_trigger.id.to_string();
+
+ let run_id = RunId::new();
+ let materialized = match state
+ .materialize_automation_run(AutomationRunMaterializeInput {
+ automation_id: automation.id.clone(),
+ target: automation.target.clone(),
+ run_id,
+ user_settings_path: state.active_config_path().to_path_buf(),
+ temp_root: automation_materialization_temp_root(state.as_ref()),
+ })
+ .await
+ {
+ Ok(materialized) => materialized,
+ Err(err) => return automation_materialize_error(&err).into_response(),
+ };
+ let explicit_title_supplied = materialized.manifest.title.is_some();
+ let automation_ref = AutomationRef {
+ id: automation.id.to_string(),
+ name: Some(automation.name.clone()),
+ trigger_id: Some(api_trigger_id),
+ };
+
+ Box::pin(runs::create_run_from_manifest(
+ state,
+ runs::CreateRunFromManifestRequest {
+ manifest: materialized.manifest,
+ submitted_manifest_bytes: materialized.submitted_manifest_bytes,
+ explicit_run_id: Some(run_id),
+ explicit_title_supplied,
+ actor,
+ headers,
+ automation: Some(automation_ref),
+ },
+ ))
+ .await
+}
+
async fn create_automation(
_auth: RequiredUser,
State(state): State<Arc<AppState>>,
@@ -130,6 +254,37 @@ fn unquote_etag(value: &str) -> &str {
.unwrap_or(value)
}
+fn enabled_api_trigger(automation: &Automation) -> Option<&ApiTrigger> {
+ if !automation.enabled {
+ return None;
+ }
+ automation
+ .triggers
+ .iter()
+ .find_map(|trigger| match trigger {
+ AutomationTrigger::Api(trigger) if trigger.enabled => Some(trigger),
+ _ => None,
+ })
+}
+
+fn automation_api_trigger_disabled_error() -> ApiError {
+ ApiError::with_code(
+ StatusCode::CONFLICT,
+ "automation is disabled or has no enabled API trigger",
+ AUTOMATION_API_TRIGGER_DISABLED_CODE,
+ )
+}
+
+fn automation_materialization_temp_root(state: &AppState) -> std::path::PathBuf {
+ Storage::new(state.server_storage_dir())
+ .scratch_dir()
+ .join("automations")
+}
+
+fn automation_materialize_error(err: &AutomationRunMaterializeError) -> ApiError {
+ ApiError::new(StatusCode::UNPROCESSABLE_ENTITY, err.to_string())
+}
+
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");
diff --git a/lib/crates/fabro-server/tests/it/api/automations.rs b/lib/crates/fabro-server/tests/it/api/automations.rs
index 37189c750..36ba160d1 100644
--- a/lib/crates/fabro-server/tests/it/api/automations.rs
+++ b/lib/crates/fabro-server/tests/it/api/automations.rs
@@ -3,11 +3,16 @@ 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 fabro_server::test_support::{
+ TestAppStateBuilder, TestAutomationRunMaterializer, build_test_router, test_auth_mode,
+};
use serde_json::{Value, json};
use tower::ServiceExt;
-use crate::helpers::{api, checked_response, response_json, response_status};
+use crate::helpers::{
+ MINIMAL_DOT, api, checked_response, minimal_manifest_json, response_json, response_status,
+ run_json,
+};
fn automation_body(id: &str, name: &str) -> Value {
json!({
@@ -66,6 +71,25 @@ fn automation_app() -> (axum::Router, tempfile::TempDir, PathBuf) {
(build_test_router(state), temp_dir, automation_dir)
}
+fn automation_app_with_fake_materializer() -> (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 materialized_manifest: fabro_api::types::RunManifest =
+ serde_json::from_value(minimal_manifest_json(MINIMAL_DOT))
+ .expect("minimal run manifest fixture should deserialize");
+ let submitted_manifest_bytes =
+ serde_json::to_vec(&materialized_manifest).expect("minimal run manifest should serialize");
+ let state = TestAppStateBuilder::new()
+ .active_config_path(active_config_path)
+ .automation_materializer(TestAutomationRunMaterializer::succeed(
+ materialized_manifest,
+ submitted_manifest_bytes,
+ ))
+ .build();
+ (build_test_router(state), temp_dir, automation_dir)
+}
+
fn json_request(method: Method, path: &str, body: &Value) -> Request<Body> {
Request::builder()
.method(method)
@@ -108,18 +132,48 @@ fn request_with_if_match(
}
async fn create_automation(app: &axum::Router, id: &str, name: &str) -> Value {
+ create_automation_with_body(app, &automation_body(id, name)).await
+}
+
+async fn create_automation_with_body(app: &axum::Router, body: &Value) -> Value {
let response = app
.clone()
- .oneshot(json_request(
- Method::POST,
- "/automations",
- &automation_body(id, name),
- ))
+ .oneshot(json_request(Method::POST, "/automations", body))
.await
.expect("create automation should respond");
response_json(response, StatusCode::CREATED, "POST /api/v1/automations").await
}
+async fn create_automation_run(
+ app: &axum::Router,
+ automation_id: &str,
+ expected: StatusCode,
+) -> Value {
+ let response = app
+ .clone()
+ .oneshot(empty_request(
+ Method::POST,
+ &format!("/automations/{automation_id}/runs"),
+ ))
+ .await
+ .expect("create automation run should respond");
+ response_json(
+ response,
+ expected,
+ format!("POST /api/v1/automations/{automation_id}/runs"),
+ )
+ .await
+}
+
+async fn list_automation_runs(app: &axum::Router, path: &str) -> Value {
+ let response = app
+ .clone()
+ .oneshot(empty_request(Method::GET, path))
+ .await
+ .expect("list automation runs should respond");
+ response_json(response, StatusCode::OK, format!("GET /api/v1{path}")).await
+}
+
fn revision_from(body: &Value) -> &str {
body["revision"]
.as_str()
@@ -499,3 +553,143 @@ async fn automations_routes_require_authenticated_user() {
)
.await;
}
+
+#[tokio::test]
+async fn disabled_automation_run_endpoint_returns_conflict_code() {
+ let (app, _temp_dir, _automation_dir) = automation_app_with_fake_materializer();
+ let mut body = automation_body("nightly", "Nightly");
+ body["enabled"] = json!(false);
+ create_automation_with_body(&app, &body).await;
+
+ let error = create_automation_run(&app, "nightly", StatusCode::CONFLICT).await;
+
+ assert_eq!(
+ error["errors"][0]["code"],
+ "automation_api_trigger_disabled"
+ );
+}
+
+#[tokio::test]
+async fn missing_automation_run_endpoint_returns_not_found() {
+ let (app, _temp_dir, _automation_dir) = automation_app_with_fake_materializer();
+
+ create_automation_run(&app, "missing", StatusCode::NOT_FOUND).await;
+}
+
+#[tokio::test]
+async fn disabled_api_trigger_run_endpoint_returns_conflict_code() {
+ let (app, _temp_dir, _automation_dir) = automation_app_with_fake_materializer();
+ let mut body = automation_body("nightly", "Nightly");
+ body["triggers"][0]["enabled"] = json!(false);
+ create_automation_with_body(&app, &body).await;
+
+ let error = create_automation_run(&app, "nightly", StatusCode::CONFLICT).await;
+
+ assert_eq!(
+ error["errors"][0]["code"],
+ "automation_api_trigger_disabled"
+ );
+}
+
+#[tokio::test]
+async fn missing_api_trigger_run_endpoint_returns_conflict_code() {
+ let (app, _temp_dir, _automation_dir) = automation_app_with_fake_materializer();
+ let mut body = automation_body("nightly", "Nightly");
+ body["triggers"] = json!([
+ {
+ "type": "schedule",
+ "id": "nightly",
+ "enabled": true,
+ "expression": "0 3 * * *"
+ }
+ ]);
+ create_automation_with_body(&app, &body).await;
+
+ let error = create_automation_run(&app, "nightly", StatusCode::CONFLICT).await;
+
+ assert_eq!(
+ error["errors"][0]["code"],
+ "automation_api_trigger_disabled"
+ );
+}
+
+#[tokio::test]
+async fn successful_api_triggered_automation_run_persists_automation_metadata() {
+ let (app, _temp_dir, _automation_dir) = automation_app_with_fake_materializer();
+ create_automation(&app, "nightly", "Nightly").await;
+
+ let created = create_automation_run(&app, "nightly", StatusCode::CREATED).await;
+
+ assert_eq!(created["automation"]["id"], "nightly");
+ assert_eq!(created["automation"]["name"], "Nightly");
+ assert_eq!(created["automation"]["trigger_id"], "manual");
+
+ let run_id = created["id"]
+ .as_str()
+ .expect("created automation run should include id");
+ let retrieved = run_json(&app, run_id).await;
+ assert_eq!(retrieved["id"], run_id);
+ assert_eq!(retrieved["automation"], created["automation"]);
+}
+
+#[tokio::test]
+async fn automation_run_listing_includes_only_runs_for_that_automation() {
+ let (app, _temp_dir, _automation_dir) = automation_app_with_fake_materializer();
+ create_automation(&app, "nightly", "Nightly").await;
+ create_automation(&app, "weekly", "Weekly").await;
+ let nightly = create_automation_run(&app, "nightly", StatusCode::CREATED).await;
+ let weekly = create_automation_run(&app, "weekly", StatusCode::CREATED).await;
+
+ let body = list_automation_runs(&app, "/automations/nightly/runs").await;
+
+ assert_eq!(body["meta"]["total"], 1);
+ assert_eq!(body["meta"]["has_more"], false);
+ assert_eq!(
+ body["data"].as_array().expect("data should be array").len(),
+ 1
+ );
+ assert_eq!(body["data"][0]["id"], nightly["id"]);
+ assert_ne!(body["data"][0]["id"], weekly["id"]);
+ assert_eq!(body["data"][0]["automation"]["id"], "nightly");
+}
+
+#[tokio::test]
+async fn missing_automation_run_listing_returns_not_found() {
+ let (app, _temp_dir, _automation_dir) = automation_app_with_fake_materializer();
+
+ let response = app
+ .oneshot(empty_request(Method::GET, "/automations/missing/runs"))
+ .await
+ .expect("missing automation run listing should respond");
+
+ response_status(
+ response,
+ StatusCode::NOT_FOUND,
+ "GET /api/v1/automations/missing/runs",
+ )
+ .await;
+}
+
+#[tokio::test]
+async fn automation_run_listing_is_newest_first_and_paginates() {
+ let (app, _temp_dir, _automation_dir) = automation_app_with_fake_materializer();
+ create_automation(&app, "nightly", "Nightly").await;
+ let oldest = create_automation_run(&app, "nightly", StatusCode::CREATED).await;
+ let middle = create_automation_run(&app, "nightly", StatusCode::CREATED).await;
+ let newest = create_automation_run(&app, "nightly", StatusCode::CREATED).await;
+
+ let first_page = list_automation_runs(&app, "/automations/nightly/runs?page[limit]=2").await;
+ assert_eq!(first_page["meta"]["total"], 3);
+ assert_eq!(first_page["meta"]["has_more"], true);
+ assert_eq!(first_page["data"][0]["id"], newest["id"]);
+ assert_eq!(first_page["data"][1]["id"], middle["id"]);
+
+ let second_page = list_automation_runs(
+ &app,
+ "/automations/nightly/runs?page[limit]=2&page[offset]=2",
+ )
+ .await;
+ assert_eq!(second_page["meta"]["total"], 3);
+ assert_eq!(second_page["meta"]["has_more"], false);
+ assert_eq!(second_page["data"][0]["id"], oldest["id"]);
+}

View file

@ -0,0 +1,6 @@
{
"outcome": "succeeded",
"notes": "Stage completed: implement",
"failure_reason": null,
"timestamp": "2026-05-28T00:42:57.609547Z"
}

View file

@ -0,0 +1,182 @@
Goal: # Issue #399: Add automation run endpoints
- URL: https://github.com/fabro-sh/fabro/issues/399
- State: OPEN
- Author: Bryan Helmkamp (@brynary)
- Created: 2026-05-25T15:06:27Z
- Updated: 2026-05-25T15:06:27Z
- Labels: None
- Assignees: None
- Milestone: None
- Comments: 0
---
## Goal
Expose API endpoints for listing runs associated with an automation and starting a run through an enabled API trigger.
## Scope
Implement these endpoints:
```http
GET /automations/{id}/runs
POST /automations/{id}/runs
```
`GET /automations/{id}/runs` behavior:
- Require the automation definition to exist; return 404 when it does not.
- List cached runs from the existing run store.
- Filter by `run.automation.as_ref().is_some_and(|a| a.id == id)`.
- Sort newest first.
- Support `page[limit]` and `page[offset]` using existing pagination behavior.
- Return the existing paginated run list envelope:
```json
{
"data": [],
"meta": { "has_more": false, "total": 0 }
}
```
`POST /automations/{id}/runs` behavior:
- Use `RequiredRunToolActor`.
- Require the automation to exist and be enabled.
- Find an enabled trigger where `type = "api"`.
- Return 409 with API error code `automation_api_trigger_disabled` when the automation is disabled or no enabled API trigger is available.
- Materialize the run manifest using the configured `AutomationRunMaterializer`.
- Call the shared create-run helper with:
```rust
AutomationRef {
id: automation.id.to_string(),
name: Some(automation.name.clone()),
trigger_id: Some(api_trigger.id.to_string()),
}
```
- Return 201 and the normal `Run` response shape with automation metadata populated.
Final integration expectations:
- Automation-created runs are visible through normal run APIs.
- Automation-created runs are visible through `GET /automations/{id}/runs`.
- Run history is derived from persisted/cached runs; no runtime automation state store is introduced.
- Schedule trigger expressions are stored and validated by earlier phases but are not scheduled by this endpoint work.
## Files
Modify:
- `lib/crates/fabro-server/src/server/handler/automations.rs`
- `lib/crates/fabro-server/src/server/handler/runs.rs`, only if additional helper exposure is needed from the previous phase
- `lib/crates/fabro-server/tests/it/api/automations.rs`
- `lib/crates/fabro-server/tests/it/api/mod.rs`
## Acceptance Criteria
- Disabled automations cannot start runs through the automation run endpoint.
- Automations without an enabled API trigger cannot start runs through the automation run endpoint.
- A successful API-triggered automation run returns a normal `Run` response with `automation.id`, `automation.name`, and `automation.trigger_id`.
- The automation run listing endpoint returns only runs linked to that automation.
- Automation run listings are newest-first and paginate correctly.
- No scheduler, web UI route/component, or CLI command is added.
## Verification
Add integration tests using the fake materializer for:
- Disabled automation returns 409.
- Disabled API trigger returns 409.
- Missing API trigger returns 409.
- Successful run creation returns 201.
- Created run persists `Run.automation`.
- Associated run listing includes the run.
- Run listing excludes runs from other automations.
- Run listing pagination and newest-first sorting.
Run:
```bash
cargo nextest run -p fabro-automation
cargo nextest run -p fabro-api
cargo nextest run -p fabro-server automations
cargo nextest run -p fabro-server openapi_conformance
cargo +nightly-2026-04-14 fmt --check --all
cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings
git diff -- apps/fabro-web lib/crates/fabro-cli
```
Expected: focused tests and checks pass; web UI and CLI command modules remain unchanged.
## Completed stages
- **toolchain**: succeeded
- 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`
- Output:
```
cargo 1.95.0 (f2d3ce0bd 2026-03-21)
```
- **preflight_compile**: succeeded
- Script: `cargo check -q --workspace 2>&1`
- Output: (empty)
- **preflight_lint**: succeeded
- Script: `cargo +nightly-2026-04-14 clippy -q --workspace --all-targets -- -D warnings 2>&1`
- Output: (empty)
- **implement**: succeeded
- Model: gpt-5.5, 1.6m tokens in / 24.4k out
# Simplify: Code Review and Cleanup
Review changes vs. origin for reuse, quality, and efficiency. Fix any issues found.
## Phase 1: Identify Changes
Run git diff (or git diff HEAD if there are staged changes) to see what changed. If there are no git changes, review the most recently modified files that the user mentioned or that you edited earlier in this conversation.
## Phase 2: Launch Three Review Agents in Parallel
Use the Agent tool to launch all three agents concurrently in a single message. Pass each agent the full diff so it has the complete context.
### Agent 1: Code Reuse Review
For each change:
1. Search for existing utilities and helpers that could replace newly written code. Use Grep to find similar patterns elsewhere in the codebase — common locations are utility directories, shared modules, and files adjacent to the changed ones.
2. Flag any new function that duplicates existing functionality. Suggest the existing function to use instead.
3. Flag any inline logic that could use an existing utility — hand-rolled string manipulation, manual path handling, custom environment checks, ad-hoc type guards, and similar patterns are common candidates.
Note: This is a greenfield app, so focus on maximizing simplicity and don't worry about changing things to achieve it.
### Agent 2: Code Quality Review
Review the same changes for hacky patterns:
1. Redundant state: state that duplicates existing state, cached values that could be derived, observers/effects that could be direct calls
2. Parameter sprawl: adding new parameters to a function instead of generalizing or restructuring existing ones
3. Copy-paste with slight variation: near-duplicate code blocks that should be unified with a shared abstraction
4. Leaky abstractions: exposing internal details that should be encapsulated, or breaking existing abstraction boundaries
5. Stringly-typed code: using raw strings where constants, enums (string unions), or branded types already exist in the codebase
Note: This is a greenfield app, so be aggressive in optimizing quality.
### Agent 3: Efficiency Review
Review the same changes for efficiency:
1. Unnecessary work: redundant computations, repeated file reads, duplicate network/API calls, N+1 patterns
2. Missed concurrency: independent operations run sequentially when they could run in parallel
3. Hot-path bloat: new blocking work added to startup or per-request/per-render hot paths
4. Unnecessary existence checks: pre-checking file/resource existence before operating (TOCTOU anti-pattern) — operate directly and handle the error
5. Memory: unbounded data structures, missing cleanup, event listener leaks
6. Overly broad operations: reading entire files when only a portion is needed, loading all items when filtering for one
## Phase 3: Fix Issues
Wait for all three agents to complete. Aggregate their findings and fix each issue directly. If a finding is a false positive or not worth addressing, note it and move on — do not argue with the finding, just skip it.
When done, briefly summarize what was fixed (or confirm the code was already clean).

View file

@ -0,0 +1,5 @@
{
"mode": "agent",
"provider": "anthropic",
"model": "claude-opus-4-7"
}

View file

@ -0,0 +1,28 @@
Web UI and CLI remain untouched.
## Summary
Reviewed 3 changed files via parallel reuse/quality/efficiency agents and applied the following cleanups:
**Removed dead/redundant code:**
- Deleted `require_run_tool_actor` helper in `principal_middleware.rs` — was a trivial alias for `require_run_management_actor`; `RequiredRunToolActor::from_request_parts` now calls the real function directly.
- Removed stale `#[allow(dead_code, reason = "...")]` on `AppState::materialize_automation_run` in `server.rs` (now called from `create_automation_run`).
**Inlined single-use helpers in `automations.rs`:**
- `automation_api_trigger_disabled_error` + the matching const `AUTOMATION_API_TRIGGER_DISABLED_CODE` — only used by one `else` branch.
- `automation_materialization_temp_root` — one-line `Storage::new(...).scratch_dir().join("automations")`.
- `automation_materialize_error` — one-line wrapper around `ApiError::new(UNPROCESSABLE_ENTITY, …)`.
**Moved logic to its proper home:**
- Promoted `enabled_api_trigger` from a private function in the HTTP handler to `Automation::enabled_api_trigger()` in the `fabro-automation` crate, where future trigger consumers (scheduler) can reuse it.
**Quality tweaks:**
- `chrono::Utc::now()``Utc::now()` via `use chrono::Utc;` to match the idiom in `runs.rs`.
- Paginate before decorate in `list_automation_runs` so `decorate_run_summaries` only runs over the returned page instead of all filtered runs.
**Considered and intentionally skipped:**
- Adding `automation_id` to `fabro_store::ListRunsQuery` to push filtering into the store (would scale better but is explicitly out of scope — issue spec mandates in-memory filtering).
- Sharing a list-runs envelope/sort helper between `list_runs` and `list_automation_runs` (would require exposing `RunsSortKey`/`RunsSortDirection` and a generic helper; the 4-line inline sort is short enough).
- Collapsing `RequiredRunToolActor` into `RequiredRunManagementActor` (issue explicitly mandates the `RequiredRunToolActor` name).
**Verification:** `cargo check --workspace --all-targets`, `cargo nextest run -p fabro-automation -p fabro-api` (176/176 pass), `cargo nextest run -p fabro-server --test it --features test-support` (171/171 pass, including all 24 automation API tests), `cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings` clean, `cargo +nightly-2026-04-14 fmt --check --all` clean. Net change: 37 inserts / 54 deletes across 4 files; `apps/fabro-web` and `lib/crates/fabro-cli` untouched.