diff --git a/crates/picloud-cli/src/cmds/pull.rs b/crates/picloud-cli/src/cmds/pull.rs index 1a7d043..ba75f4d 100644 --- a/crates/picloud-cli/src/cmds/pull.rs +++ b/crates/picloud-cli/src/cmds/pull.rs @@ -47,6 +47,20 @@ pub async fn run(app_ident: &str, dir: &Path, force: bool, mode: OutputMode) -> let scripts = client.scripts_list_by_app(app_ident).await?; let triggers = client.triggers_list(app_ident).await?.triggers; let workflows = client.workflows_list(app_ident).await?; + // A live workflow always has ≥1 step (a zero-step definition is rejected at + // apply), so an empty `definition` here can only mean the server predates + // the definition-in-list field. Fail fast BEFORE writing anything — silently + // dropping the workflow would leave a manifest that a later `apply --prune` + // uses to DELETE the server-side workflow (and `pull --force` would clobber + // a hand-authored block). Mirror the clobber/unsafe-name bails above. + if let Some(w) = workflows.iter().find(|w| w.definition.steps.is_empty()) { + anyhow::bail!( + "workflow `{}` came back without its definition — the server is too old to \ + round-trip workflows via `pull`. Refusing to write a manifest that omits it \ + (a later `apply --prune` would delete the workflow). Upgrade the server and retry.", + w.name + ); + } let secrets = client .secrets_list(VarOwnerArg::App(app_ident), None) .await? @@ -253,10 +267,7 @@ pub async fn run(app_ident: &str, dir: &Path, force: bool, mode: OutputMode) -> }, vars: manifest_vars, suppress: crate::manifest::ManifestSuppress::default(), - workflows: workflows - .iter() - .filter_map(wire_workflow_to_manifest) - .collect(), + workflows: workflows.iter().map(wire_workflow_to_manifest).collect(), }; std::fs::write(&manifest_path, manifest.to_toml()?) @@ -284,20 +295,11 @@ fn trigger_count(t: &ManifestTriggers) -> usize { /// drop the app's workflows, and a subsequent `pic apply --prune` would delete /// them from the server. /// -/// Returns `None` (with a warning) when the definition came back empty. A -/// zero-step workflow is rejected at apply, so an empty `definition` can only -/// mean the server predates the definition-in-list field (version skew) — -/// emitting a stepless block would just fail the next apply, so skip it. -fn wire_workflow_to_manifest(w: &crate::client::WorkflowDto) -> Option { - if w.definition.steps.is_empty() { - eprintln!( - "warning: skipping workflow `{}` — the server did not return its \ - definition (upgrade the server to round-trip workflows via pull)", - w.name - ); - return None; - } - Some(ManifestWorkflow { +/// The caller guarantees `definition.steps` is non-empty (an empty definition +/// means an out-of-date server and is rejected up front in `run`, before any +/// file is written). +fn wire_workflow_to_manifest(w: &crate::client::WorkflowDto) -> ManifestWorkflow { + ManifestWorkflow { name: w.name.clone(), enabled: w.enabled, steps: w @@ -306,7 +308,7 @@ fn wire_workflow_to_manifest(w: &crate::client::WorkflowDto) -> Option ManifestWorkflowStep {