Adversarial review of the v1.2 Workflows track surfaced two HIGH footguns and several correctness/hardening gaps. Fixes: HIGH - pull round-trip: `pic pull` dropped `[[workflows]]`, so a later `apply --prune` silently deleted them. The list endpoint now returns the full `definition`; pull rebuilds the manifest block (inverse of workflow_to_wire). - case-colliding names: two workflows differing only by case collided in the reconcile diff (keyed by lower(name)), silently dropping one. Rejected up front in validate_bundle_for. MEDIUM - reclaim retry budget: a crashed attempt (no outcome) consumed the retry budget. reclaim_stale_steps now decrements `attempt` (floored) and clears `next_attempt_at`, so a crash no longer counts as a failed try. - on_error/backoff: typed the manifest fields against the shared enums so a bad value fails at TOML parse with a clear message, not an opaque 500. - dedupe depends_on: a repeated dependency inflated the Kahn in-degree past the single decrement, reporting a spurious cycle for a valid DAG. Count distinct. - canceled child: a canceled sub-workflow resolved the parent step with a message naming the cause instead of a generic "failed"; documented that a run-level cancel op is not yet supported. LOW - list_run_steps is now app-scoped at the query (JOIN workflow_runs) rather than relying on caller pre-verification. - partial failure is surfaced: a run that succeeds with on_error=continue failures records them in the run's `error` field. - admin-started runs log the root_execution_id against the principal. - documented the deliberate when(missing→false) vs template(missing→fail) asymmetry; corrected the claim's atomicity comment. Tests: unit (dedupe-deps), DB-gated reclaim-budget assertion, and two new CLI journeys (duplicate-name rejection, pull→plan clean round-trip). fmt + clippy -D warnings clean; 450 lib + 14 orchestrator DB + 5 workflow journeys pass; schema snapshot unchanged (no migration). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
333 lines
11 KiB
Rust
333 lines
11 KiB
Rust
//! Admin API for v1.2 Workflows (M5): list definitions, start runs, and read
|
|
//! run history.
|
|
//!
|
|
//! - `GET /apps/{app}/workflows` — list the app's workflows
|
|
//! - `POST /apps/{app}/workflows/{name}/runs` — start a run (body: input JSON)
|
|
//! - `GET /apps/{app}/workflows/{name}/runs` — recent runs of a workflow
|
|
//! - `GET /apps/{app}/workflow-runs/{run_id}` — one run + its steps
|
|
//!
|
|
//! Reads are gated on `AppRead`; starting a run is gated on `AppInvoke` — the
|
|
//! same capability the SDK `workflow::start` uses. `app_id` scopes every query
|
|
//! (the isolation boundary); a run of another app 404s.
|
|
|
|
use std::sync::Arc;
|
|
|
|
use axum::extract::{Path, State};
|
|
use axum::http::StatusCode;
|
|
use axum::response::{IntoResponse, Json, Response};
|
|
use axum::routing::get;
|
|
use axum::{Extension, Router};
|
|
use chrono::{DateTime, Utc};
|
|
use picloud_shared::{AppId, Principal, WorkflowDefinition, WorkflowRunId};
|
|
use serde::{Deserialize, Serialize};
|
|
use serde_json::{json, Value};
|
|
use uuid::Uuid;
|
|
|
|
use crate::app_repo::AppRepository;
|
|
use crate::authz::{require, AuthzDenied, AuthzError, AuthzRepo, Capability};
|
|
use crate::workflow_repo::{
|
|
get_run, get_workflow_by_id, get_workflow_by_name, list_run_steps, list_runs_for_workflow,
|
|
list_workflows_for_app, start_run, NewWorkflowRun, WorkflowRun, WorkflowRunStep,
|
|
};
|
|
use sqlx::PgPool;
|
|
|
|
/// Cap on the run-history list length.
|
|
const RUN_LIST_LIMIT: i64 = 100;
|
|
|
|
#[derive(Clone)]
|
|
pub struct WorkflowsState {
|
|
pub pool: PgPool,
|
|
pub apps: Arc<dyn AppRepository>,
|
|
pub authz: Arc<dyn AuthzRepo>,
|
|
}
|
|
|
|
pub fn workflows_router(state: WorkflowsState) -> Router {
|
|
Router::new()
|
|
.route("/apps/{app_id}/workflows", get(list_workflows))
|
|
.route(
|
|
"/apps/{app_id}/workflows/{name}/runs",
|
|
get(list_runs).post(start_run_handler),
|
|
)
|
|
.route("/apps/{app_id}/workflow-runs/{run_id}", get(get_run_detail))
|
|
.with_state(state)
|
|
}
|
|
|
|
// ---- response DTOs --------------------------------------------------------
|
|
|
|
#[derive(Serialize)]
|
|
struct WorkflowDto {
|
|
name: String,
|
|
enabled: bool,
|
|
/// Step count (kept for the dashboard's summary column).
|
|
steps: usize,
|
|
/// The full stored definition, so `pic pull` can round-trip the
|
|
/// `[[workflows]]` manifest block (without it, a pull-then-apply silently
|
|
/// deletes every workflow).
|
|
definition: WorkflowDefinition,
|
|
}
|
|
|
|
#[derive(Serialize)]
|
|
struct RunDto {
|
|
id: Uuid,
|
|
workflow_id: Uuid,
|
|
status: String,
|
|
input: Value,
|
|
output: Option<Value>,
|
|
error: Option<String>,
|
|
workflow_depth: i32,
|
|
root_execution_id: Uuid,
|
|
created_at: DateTime<Utc>,
|
|
started_at: Option<DateTime<Utc>>,
|
|
finished_at: Option<DateTime<Utc>>,
|
|
}
|
|
|
|
impl From<WorkflowRun> for RunDto {
|
|
fn from(r: WorkflowRun) -> Self {
|
|
Self {
|
|
id: r.id.into_inner(),
|
|
workflow_id: r.workflow_id.into_inner(),
|
|
status: r.status.as_str().to_string(),
|
|
input: r.input,
|
|
output: r.output,
|
|
error: r.error,
|
|
workflow_depth: r.workflow_depth,
|
|
root_execution_id: r.root_execution_id,
|
|
created_at: r.created_at,
|
|
started_at: r.started_at,
|
|
finished_at: r.finished_at,
|
|
}
|
|
}
|
|
}
|
|
|
|
#[derive(Serialize)]
|
|
struct StepDto {
|
|
name: String,
|
|
status: String,
|
|
attempt: i32,
|
|
max_attempts: i32,
|
|
output: Option<Value>,
|
|
error: Option<String>,
|
|
child_run_id: Option<Uuid>,
|
|
/// DAG edges (from the definition) so the dashboard can draw the graph.
|
|
depends_on: Vec<String>,
|
|
}
|
|
|
|
impl StepDto {
|
|
fn from_step(s: WorkflowRunStep, depends_on: Vec<String>) -> Self {
|
|
Self {
|
|
name: s.step_name,
|
|
status: s.status.as_str().to_string(),
|
|
attempt: s.attempt,
|
|
max_attempts: s.max_attempts,
|
|
output: s.output,
|
|
error: s.error,
|
|
child_run_id: s.child_run_id.map(WorkflowRunId::into_inner),
|
|
depends_on,
|
|
}
|
|
}
|
|
}
|
|
|
|
#[derive(Deserialize, Default)]
|
|
struct StartRunReq {
|
|
#[serde(default)]
|
|
input: Value,
|
|
}
|
|
|
|
// ---- handlers -------------------------------------------------------------
|
|
|
|
async fn list_workflows(
|
|
State(s): State<WorkflowsState>,
|
|
Extension(principal): Extension<Principal>,
|
|
Path(id_or_slug): Path<String>,
|
|
) -> Result<Json<Vec<WorkflowDto>>, WorkflowsApiError> {
|
|
let app_id = resolve_app(&*s.apps, &id_or_slug).await?;
|
|
require(s.authz.as_ref(), &principal, Capability::AppRead(app_id)).await?;
|
|
let workflows = list_workflows_for_app(&s.pool, app_id)
|
|
.await
|
|
.map_err(|e| WorkflowsApiError::Backend(e.to_string()))?;
|
|
Ok(Json(
|
|
workflows
|
|
.into_iter()
|
|
.map(|w| WorkflowDto {
|
|
name: w.name,
|
|
enabled: w.enabled,
|
|
steps: w.definition.steps.len(),
|
|
definition: w.definition,
|
|
})
|
|
.collect(),
|
|
))
|
|
}
|
|
|
|
async fn start_run_handler(
|
|
State(s): State<WorkflowsState>,
|
|
Extension(principal): Extension<Principal>,
|
|
Path((id_or_slug, name)): Path<(String, String)>,
|
|
body: Option<Json<StartRunReq>>,
|
|
) -> Result<(StatusCode, Json<RunDto>), WorkflowsApiError> {
|
|
let app_id = resolve_app(&*s.apps, &id_or_slug).await?;
|
|
require(s.authz.as_ref(), &principal, Capability::AppInvoke(app_id)).await?;
|
|
|
|
let wf = get_workflow_by_name(&s.pool, app_id, &name)
|
|
.await
|
|
.map_err(|e| WorkflowsApiError::Backend(e.to_string()))?
|
|
.ok_or_else(|| WorkflowsApiError::NotFound(format!("workflow {name:?}")))?;
|
|
if !wf.enabled {
|
|
return Err(WorkflowsApiError::Invalid(format!(
|
|
"workflow {name:?} is disabled"
|
|
)));
|
|
}
|
|
let input = body.map_or(Value::Null, |Json(b)| b.input);
|
|
// An operator-initiated run has no parent execution to correlate to, so it
|
|
// roots a fresh execution tree. Log the id against the principal so the
|
|
// run's step logs remain traceable to who launched it (the SDK path threads
|
|
// `cx.root_execution_id` instead).
|
|
let root_execution_id = Uuid::new_v4();
|
|
let new = NewWorkflowRun {
|
|
workflow_id: wf.id,
|
|
app_id,
|
|
input,
|
|
root_execution_id,
|
|
workflow_depth: 0,
|
|
parent_run_id: None,
|
|
parent_step_id: None,
|
|
};
|
|
let run_id = start_run(&s.pool, new, &wf.definition)
|
|
.await
|
|
.map_err(|e| WorkflowsApiError::Backend(e.to_string()))?;
|
|
tracing::info!(
|
|
%app_id, workflow = %name, run_id = %run_id.into_inner(), %root_execution_id,
|
|
user_id = %principal.user_id.into_inner(),
|
|
"admin started a workflow run"
|
|
);
|
|
// Read it back for the response (status = pending until the orchestrator ticks).
|
|
let run = get_run(&s.pool, app_id, run_id)
|
|
.await
|
|
.map_err(|e| WorkflowsApiError::Backend(e.to_string()))?
|
|
.ok_or_else(|| WorkflowsApiError::Backend("run vanished after start".into()))?;
|
|
Ok((StatusCode::CREATED, Json(run.into())))
|
|
}
|
|
|
|
async fn list_runs(
|
|
State(s): State<WorkflowsState>,
|
|
Extension(principal): Extension<Principal>,
|
|
Path((id_or_slug, name)): Path<(String, String)>,
|
|
) -> Result<Json<Vec<RunDto>>, WorkflowsApiError> {
|
|
let app_id = resolve_app(&*s.apps, &id_or_slug).await?;
|
|
require(s.authz.as_ref(), &principal, Capability::AppRead(app_id)).await?;
|
|
let wf = get_workflow_by_name(&s.pool, app_id, &name)
|
|
.await
|
|
.map_err(|e| WorkflowsApiError::Backend(e.to_string()))?
|
|
.ok_or_else(|| WorkflowsApiError::NotFound(format!("workflow {name:?}")))?;
|
|
let runs = list_runs_for_workflow(&s.pool, app_id, wf.id, RUN_LIST_LIMIT)
|
|
.await
|
|
.map_err(|e| WorkflowsApiError::Backend(e.to_string()))?;
|
|
Ok(Json(runs.into_iter().map(Into::into).collect()))
|
|
}
|
|
|
|
async fn get_run_detail(
|
|
State(s): State<WorkflowsState>,
|
|
Extension(principal): Extension<Principal>,
|
|
Path((id_or_slug, run_id)): Path<(String, Uuid)>,
|
|
) -> Result<Json<Value>, WorkflowsApiError> {
|
|
let app_id = resolve_app(&*s.apps, &id_or_slug).await?;
|
|
require(s.authz.as_ref(), &principal, Capability::AppRead(app_id)).await?;
|
|
let run = get_run(&s.pool, app_id, WorkflowRunId::from(run_id))
|
|
.await
|
|
.map_err(|e| WorkflowsApiError::Backend(e.to_string()))?
|
|
.ok_or_else(|| WorkflowsApiError::NotFound(format!("run {run_id}")))?;
|
|
let steps = list_run_steps(&s.pool, app_id, run.id)
|
|
.await
|
|
.map_err(|e| WorkflowsApiError::Backend(e.to_string()))?;
|
|
// Load the definition for the per-step `depends_on` DAG edges (they live on
|
|
// the definition, not the run's step rows).
|
|
let deps: std::collections::HashMap<String, Vec<String>> =
|
|
match get_workflow_by_id(&s.pool, app_id, run.workflow_id)
|
|
.await
|
|
.map_err(|e| WorkflowsApiError::Backend(e.to_string()))?
|
|
{
|
|
Some(w) => w
|
|
.definition
|
|
.steps
|
|
.into_iter()
|
|
.map(|st| (st.name, st.depends_on))
|
|
.collect(),
|
|
None => std::collections::HashMap::new(),
|
|
};
|
|
let run_dto: RunDto = run.into();
|
|
let step_dtos: Vec<StepDto> = steps
|
|
.into_iter()
|
|
.map(|st| {
|
|
let d = deps.get(&st.step_name).cloned().unwrap_or_default();
|
|
StepDto::from_step(st, d)
|
|
})
|
|
.collect();
|
|
Ok(Json(json!({ "run": run_dto, "steps": step_dtos })))
|
|
}
|
|
|
|
async fn resolve_app(apps: &dyn AppRepository, ident: &str) -> Result<AppId, WorkflowsApiError> {
|
|
crate::app_repo::resolve_app(apps, ident)
|
|
.await
|
|
.map_err(|e| WorkflowsApiError::Backend(e.to_string()))?
|
|
.map(|l| l.app.id)
|
|
.ok_or_else(|| WorkflowsApiError::NotFound(format!("app {ident:?}")))
|
|
}
|
|
|
|
// ---- errors ---------------------------------------------------------------
|
|
|
|
#[derive(Debug, thiserror::Error)]
|
|
pub enum WorkflowsApiError {
|
|
#[error("{0}")]
|
|
NotFound(String),
|
|
#[error("{0}")]
|
|
Invalid(String),
|
|
#[error("forbidden")]
|
|
Forbidden,
|
|
#[error("authorization repo error: {0}")]
|
|
AuthzRepo(String),
|
|
#[error("workflow backend: {0}")]
|
|
Backend(String),
|
|
}
|
|
|
|
impl From<AuthzDenied> for WorkflowsApiError {
|
|
fn from(d: AuthzDenied) -> Self {
|
|
match d {
|
|
AuthzDenied::Denied => Self::Forbidden,
|
|
AuthzDenied::Repo(e) => Self::AuthzRepo(e.to_string()),
|
|
}
|
|
}
|
|
}
|
|
|
|
impl From<AuthzError> for WorkflowsApiError {
|
|
fn from(e: AuthzError) -> Self {
|
|
Self::AuthzRepo(e.to_string())
|
|
}
|
|
}
|
|
|
|
impl IntoResponse for WorkflowsApiError {
|
|
fn into_response(self) -> Response {
|
|
let (status, body) = match &self {
|
|
Self::NotFound(_) => (StatusCode::NOT_FOUND, json!({ "error": self.to_string() })),
|
|
Self::Invalid(_) => (
|
|
StatusCode::UNPROCESSABLE_ENTITY,
|
|
json!({ "error": self.to_string() }),
|
|
),
|
|
Self::Forbidden => (StatusCode::FORBIDDEN, json!({ "error": self.to_string() })),
|
|
Self::AuthzRepo(e) => {
|
|
tracing::error!(error = %e, "workflows authz repo error");
|
|
(
|
|
StatusCode::INTERNAL_SERVER_ERROR,
|
|
json!({ "error": "internal error" }),
|
|
)
|
|
}
|
|
Self::Backend(e) => {
|
|
tracing::error!(error = %e, "workflows api backend error");
|
|
(
|
|
StatusCode::INTERNAL_SERVER_ERROR,
|
|
json!({ "error": "internal error" }),
|
|
)
|
|
}
|
|
};
|
|
(status, Json(body)).into_response()
|
|
}
|
|
}
|