feat(workflow): distinguish plan-only builds to ensure safe CI execution

josh-tracey · josh-tracey · commit 51e5fe2e45b6 · 2026-07-08T20:19:56.000+01:00
diff --git a/.gitignore b/.gitignore
@@ -7,3 +7,4 @@ k8s/environments/dev
 .kube
 netrc
 .roomservice/
+.sailr/
diff --git a/examples/ci-build/hello/Dockerfile b/examples/ci-build/hello/Dockerfile
@@ -0,0 +1,2 @@
+FROM nginx:alpine
+COPY index.html /usr/share/nginx/html/index.html
diff --git a/examples/ci-build/hello/index.html b/examples/ci-build/hello/index.html
@@ -0,0 +1,5 @@
+<!DOCTYPE html>
+<html>
+<head><title>CI Build</title></head>
+<body><h1>Hello from CI Build!</h1></body>
+</html>
diff --git a/hello-sailr/Dockerfile b/hello-sailr/Dockerfile
@@ -0,0 +1 @@
+FROM scratch
diff --git a/k8s/environments/local/config.toml b/k8s/environments/local/config.toml
@@ -10,6 +10,14 @@ name = "hello-sailr"
 version = "latest"
 path = "hello-sailr"
 
+
+[[service]]
+name = "ci-build-hello"
+version = "ci"
+path = "examples/ci-build/hello"
+
 [service.build]
-path = "."
-dockerfile = "Dockerfile"
+path = "examples/ci-build/hello"
+include = ["Dockerfile", "index.html"]
+build_command = "docker build -t hello-sailr:ci ."
+push_command = "true"
diff --git a/k8s/generated/local/examples/ci-build/hello/deployment.yaml b/k8s/generated/local/examples/ci-build/hello/deployment.yaml
diff --git a/k8s/generated/local/examples/ci-build/hello/service.yaml b/k8s/generated/local/examples/ci-build/hello/service.yaml
diff --git a/k8s/templates/examples/ci-build/hello/deployment.yaml b/k8s/templates/examples/ci-build/hello/deployment.yaml
diff --git a/k8s/templates/examples/ci-build/hello/service.yaml b/k8s/templates/examples/ci-build/hello/service.yaml
diff --git a/sailr.workflow.toml b/sailr.workflow.toml
@@ -3,6 +3,33 @@ environment = "local"
 mode = "check"
 interactive = false
 build = "plan"
+generate = "disabled"
+deploy = "disabled"
+report = "text"
+
+[workflow.ci-generate]
+environment = "local"
+mode = "check"
+interactive = false
+build = "plan"
 generate = "run"
 deploy = "disabled"
 report = "text"
+
+[workflow.ci-build-plan]
+environment = "local"
+mode = "check"
+interactive = false
+build = "plan"
+generate = "disabled"
+deploy = "disabled"
+report = "text"
+
+[workflow.ci-build]
+environment = "local"
+mode = "build"
+interactive = false
+build = "run"
+generate = "disabled"
+deploy = "disabled"
+report = "text"
diff --git a/src/workflow/planner.rs b/src/workflow/planner.rs
@@ -7,6 +7,12 @@ use super::profile::NormalizedWorkflowProfile;
 
 use std::sync::Arc;
 
+pub enum WorkflowBuildExecution {
+    None,
+    PlanOnly(SailrBuildPlan),
+    Executed(SailrBuildPlan),
+}
+
 pub struct WorkflowPlanner {
     pub profile: NormalizedWorkflowProfile,
     pub env: Arc<Environment>,
@@ -26,9 +32,9 @@ impl WorkflowPlanner {
         }
     }
 
-    pub fn build_pipeline(&self) -> Result<(Pipeline, Option<SailrBuildPlan>), String> {
+    pub fn build_pipeline(&self) -> Result<(Pipeline, WorkflowBuildExecution), String> {
         let mut pipeline = Pipeline::new(format!("Workflow: {}", self.profile.name));
-        let mut build_plan = None;
+        let mut build_execution = WorkflowBuildExecution::None;
 
         // 0. Validate Phase
 
@@ -40,15 +46,13 @@ impl WorkflowPlanner {
         let mut last_tasks: Vec<String> = vec!["workflow:validate-config".to_string()];
 
         // 1. Build Phase
-        if self.profile.build.is_active() {
-            let plan = create_sailr_build_plan(&self.env, &self.options)?;
-
-            // Only add runkernel tasks if we actually want to run the build.
-            // If build == Plan, create_sailr_build_plan already printed the plan (via builder.rs integration),
-            // but we don't want to execute it. Wait, create_sailr_build_plan does not print the plan.
-            // In RunkernelBuildBackend::build, it prints it.
-            // But we can just avoid adding tasks to the pipeline if it's just plan.
-            if self.profile.build == crate::workflow::profile::WorkflowStepMode::Plan {
+        match self.profile.build {
+            crate::workflow::profile::WorkflowStepMode::Disabled => {}
+            crate::workflow::profile::WorkflowStepMode::DryRun => {
+                return Err("workflow build dry-run is not enabled in this PR".to_string());
+            }
+            crate::workflow::profile::WorkflowStepMode::Plan => {
+                let plan = create_sailr_build_plan(&self.env, &self.options)?;
                 let plan_for_task = plan.clone();
                 let options_for_task = self.options.clone();
 
@@ -65,8 +69,10 @@ impl WorkflowPlanner {
 
                 pipeline.add(task);
                 last_tasks = vec!["workflow:build-plan".to_string()];
-                build_plan = Some(plan);
-            } else {
+                build_execution = WorkflowBuildExecution::PlanOnly(plan);
+            }
+            crate::workflow::profile::WorkflowStepMode::Run => {
+                let plan = create_sailr_build_plan(&self.env, &self.options)?;
                 add_runkernel_tasks(&mut pipeline, &plan)?;
 
                 let dirty_count = plan.services.iter().filter(|s| s.dirty).count();
@@ -80,7 +86,7 @@ impl WorkflowPlanner {
                 }
 
                 last_tasks.extend(build_deps);
-                build_plan = Some(plan);
+                build_execution = WorkflowBuildExecution::Executed(plan);
             }
         }
 
@@ -123,7 +129,7 @@ impl WorkflowPlanner {
             // last_tasks = vec!["workflow:generate".to_string()];
         }
 
-        Ok((pipeline, build_plan))
+        Ok((pipeline, build_execution))
     }
 }
 
@@ -217,4 +223,20 @@ mod tests {
         let (pipeline, _) = planner.build_pipeline().unwrap();
         assert!(pipeline.tasks().any(|t| t.name == "workflow:generate"));
     }
+
+    #[test]
+    fn generate_depends_on_build_plan() {
+        let env = Environment::load_from_file("local").unwrap();
+        let profile = dummy_profile(WorkflowStepMode::Disabled, WorkflowStepMode::Plan);
+        let planner = WorkflowPlanner::new(profile, Arc::new(env), dummy_options(true));
+        let (pipeline, _) = planner.build_pipeline().unwrap();
+
+        let generate_task = pipeline
+            .tasks()
+            .find(|t| t.name == "workflow:generate")
+            .unwrap();
+        assert!(generate_task
+            .dependencies
+            .contains(&"workflow:build-plan".to_string()));
+    }
 }
diff --git a/src/workflow/profile.rs b/src/workflow/profile.rs
@@ -683,14 +683,14 @@ mod tests {
         "#;
         let profile: WorkflowProfile = toml::from_str(toml_str).unwrap();
         let normalized = profile.normalize(false);
-        assert_eq!(normalized.interactive, false);
+        assert!(!normalized.interactive);
         assert_eq!(normalized.build, WorkflowStepMode::Plan);
         assert_eq!(normalized.generate, WorkflowStepMode::Run);
         assert_eq!(normalized.deploy, WorkflowStepMode::Disabled);
         assert_eq!(normalized.test, WorkflowStepMode::Disabled);
         assert_eq!(normalized.verify, WorkflowStepMode::Disabled);
         assert_eq!(normalized.approval, ApprovalMode::None);
-        assert_eq!(normalized.apply, false);
+        assert!(!normalized.apply);
     }
 
     #[test]
@@ -705,6 +705,6 @@ mod tests {
         assert_eq!(normalized.generate, WorkflowStepMode::Disabled);
         assert_eq!(normalized.deploy, WorkflowStepMode::Disabled);
         assert_eq!(normalized.approval, ApprovalMode::None);
-        assert_eq!(normalized.apply, false);
+        assert!(!normalized.apply);
     }
 }
diff --git a/src/workflow/runner.rs b/src/workflow/runner.rs
@@ -46,7 +46,18 @@ impl RunnerContext {
     }
 }
 
-fn validate_workflow_safety(
+fn print_failed_tasks(result: &runkernel::PipelineResult) {
+    for task in &result.tasks {
+        if matches!(task.status, runkernel::TaskStatus::Failed) {
+            println!("Failed task: {}", task.name);
+            if let Some(error) = &task.error {
+                println!("  error: {}", error);
+            }
+        }
+    }
+}
+
+pub fn validate_workflow_safety(
     profile: &crate::workflow::profile::NormalizedWorkflowProfile,
     runner: &RunnerContext,
 ) -> Result<(), String> {
@@ -126,7 +137,7 @@ impl WorkflowRunner {
             std::sync::Arc::new(env),
             options,
         );
-        let (mut pipeline, build_plan) = planner.build_pipeline()?;
+        let (mut pipeline, build_execution) = planner.build_pipeline()?;
 
         // 8. Run Pipeline
         attach_pipeline_logging(&mut pipeline);
@@ -141,14 +152,21 @@ impl WorkflowRunner {
             .map_err(|e| format!("Pipeline execution failed: {:?}", e))?;
 
         // 9. Finalize
-        if let Some(plan) = build_plan {
-            crate::builder::print_pipeline_result(&plan, &result);
-            if result.summary.success {
-                write_successful_service_caches(&plan, &result)?;
+        match build_execution {
+            crate::workflow::planner::WorkflowBuildExecution::None => {}
+            crate::workflow::planner::WorkflowBuildExecution::PlanOnly(_) => {
+                // Do not print build results or write caches
+            }
+            crate::workflow::planner::WorkflowBuildExecution::Executed(plan) => {
+                crate::builder::print_pipeline_result(&plan, &result);
+                if result.summary.success {
+                    write_successful_service_caches(&plan, &result)?;
+                }
             }
         }
 
         if !result.summary.success {
+            print_failed_tasks(&result);
             return Err(format!(
                 "Workflow failed: {} failed, {} skipped, {} cancelled",
                 result.summary.failed, result.summary.skipped, result.summary.cancelled

-Original file line number
+Diff line change
 .kube
 netrc
 .roomservice/
 +.sailr/
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+FROM nginx:alpine`
	`2`	`+COPY index.html /usr/share/nginx/html/index.html`