Store Pipeline in Runtime and return outputs from execute
Runtime wraps the whole Pipeline instead of decomposing it for
individual fields. Run::execute returns a HashMap of job outputs
directly, removing the need for Run::outputs and the persistent
runtime field.

Assisted-by: GLM-5.1 via pi
change tuylzqupnqvpzsnxynnnqqksklyrwswt
commit db0abdb83d8b006458497abae40e809ea844040a
author Alpha Chen <alpha@kejadlen.dev>
date
parent nxqsrxnl
diff --git a/src/bin/quire/commands/ci.rs b/src/bin/quire/commands/ci.rs
index 9c32c77..c1c9507 100644
--- a/src/bin/quire/commands/ci.rs
+++ b/src/bin/quire/commands/ci.rs
@@ -73,29 +73,28 @@ pub async fn run(quire: &Quire, maybe_sha: Option<&str>) -> Result<()> {
         pushed_at: jiff::Timestamp::now(),
     };
 
-    let job_ids: Vec<String> = pipeline.jobs().iter().map(|j| j.id.clone()).collect();
-
     let mut run = runs.create(&meta)?;
     println!("Run {}: executing at {}", run.id(), commit.display);
 
     let exec_result = run.execute(pipeline, secrets);
 
-    // Pipeline was consumed by execute. Output display uses run.outputs()
-    for job_id in &job_ids {
-        let outputs = run.outputs(job_id);
-        if outputs.is_empty() {
-            continue;
-        }
-        println!("\n==> {}", job_id);
-        for o in &outputs {
-            if !o.stdout.is_empty() {
-                print!("{}", o.stdout);
-            }
-            if !o.stderr.is_empty() {
-                eprint!("{}", o.stderr);
+    // Display outputs from completed jobs.
+    if let Ok(ref outputs) = exec_result {
+        for (job_id, job_outputs) in outputs {
+            if job_outputs.is_empty() {
+                continue;
             }
-            if o.exit != 0 {
-                println!("(exit {})", o.exit);
+            println!("\n==> {}", job_id);
+            for o in job_outputs {
+                if !o.stdout.is_empty() {
+                    print!("{}", o.stdout);
+                }
+                if !o.stderr.is_empty() {
+                    eprint!("{}", o.stderr);
+                }
+                if o.exit != 0 {
+                    println!("(exit {})", o.exit);
+                }
             }
         }
     }
@@ -111,7 +110,7 @@ pub async fn run(quire: &Quire, maybe_sha: Option<&str>) -> Result<()> {
         }
         (state, result) => {
             println!("\nRun ended in unexpected state: {state:?}");
-            result.into_diagnostic()
+            result.map(|_| ()).into_diagnostic()
         }
     }
 }
diff --git a/src/ci/lua.rs b/src/ci/lua.rs
index fccd1cc..82ef949 100644
--- a/src/ci/lua.rs
+++ b/src/ci/lua.rs
@@ -8,7 +8,7 @@
 //! each `run-fn` at execute time.
 
 use std::cell::RefCell;
-use std::collections::{HashMap, HashSet};
+use std::collections::HashMap;
 use std::rc::Rc;
 
 use mlua::{IntoLua, Lua, LuaSerdeExt};
@@ -116,7 +116,7 @@ fn register_job(
 /// `(secret …)` and `(jobs …)` require a runtime — without one, calls
 /// error.
 pub(super) struct Runtime {
-    fennel: Fennel,
+    pipeline: super::pipeline::Pipeline,
     secrets: HashMap<String, SecretString>,
     inputs: HashMap<String, HashMap<String, Option<mlua::Value>>>,
     current_job: RefCell<Option<String>>,
@@ -124,18 +124,17 @@ pub(super) struct Runtime {
 }
 
 impl Runtime {
-    /// Build a fresh runtime owning `fennel` (the Lua VM).
+    /// Consume `pipeline` and build a runtime ready to execute it.
     ///
-    /// `meta` provides the push data for `:quire/push` source outputs.
-    /// `transitive` maps each job to its set of reachable input names;
-    /// the runtime builds per-job views from this and the source values.
+    /// Takes ownership of the pipeline (including its Lua VM). `meta`
+    /// provides the push data for `:quire/push` source outputs.
     pub(super) fn new(
-        fennel: Fennel,
+        pipeline: super::pipeline::Pipeline,
         secrets: HashMap<String, SecretString>,
         meta: &super::run::RunMeta,
-        transitive: &HashMap<String, HashSet<String>>,
     ) -> Self {
-        let lua = fennel.lua();
+        let transitive = pipeline.transitive_inputs();
+        let lua = pipeline.fennel().lua();
 
         // Build the push outputs as a Lua table.
         let push = lua.create_table().expect("create push table");
@@ -147,7 +146,7 @@ impl Runtime {
 
         // Build per-job input views from transitive reachability.
         let mut inputs = HashMap::new();
-        for (job_id, reachable) in transitive {
+        for (job_id, reachable) in &transitive {
             let mut view = HashMap::new();
             for name in reachable {
                 let value = if name == "quire/push" {
@@ -161,7 +160,7 @@ impl Runtime {
         }
 
         Self {
-            fennel,
+            pipeline,
             secrets,
             inputs,
             current_job: RefCell::new(None),
@@ -171,7 +170,17 @@ impl Runtime {
 
     /// Borrow the underlying Lua VM.
     pub(super) fn lua(&self) -> &Lua {
-        self.fennel.lua()
+        self.pipeline.fennel().lua()
+    }
+
+    /// The topo-sorted job IDs in execution order.
+    pub(super) fn topo_order(&self) -> Vec<&str> {
+        self.pipeline.topo_order()
+    }
+
+    /// Look up a job by id.
+    pub(super) fn job(&self, id: &str) -> Option<&super::pipeline::Job> {
+        self.pipeline.job(id)
     }
 
     /// Mark `id` as the currently executing job. `(sh …)` invocations
@@ -187,19 +196,22 @@ impl Runtime {
         *self.current_job.borrow_mut() = None;
     }
 
-    /// Snapshot the recorded outputs for `id`. Empty if the job
-    /// produced none (or hasn't run).
-    pub(super) fn outputs(&self, id: &str) -> Vec<ShOutput> {
-        self.outputs.borrow().get(id).cloned().unwrap_or_default()
+    /// Drain all recorded outputs, returning them keyed by job id.
+    pub(super) fn take_outputs(&self) -> HashMap<String, Vec<ShOutput>> {
+        std::mem::take(&mut *self.outputs.borrow_mut())
     }
 }
 
 #[cfg(test)]
 impl Runtime {
-    /// Minimal constructor for tests — no inputs, no source outputs.
-    fn for_test(fennel: Fennel, secrets: HashMap<String, SecretString>) -> Self {
+    /// Minimal constructor for tests — no source outputs, just
+    /// secrets and the pipeline's VM.
+    fn for_test(
+        pipeline: super::pipeline::Pipeline,
+        secrets: HashMap<String, SecretString>,
+    ) -> Self {
         Self {
-            fennel,
+            pipeline,
             secrets,
             inputs: HashMap::new(),
             current_job: RefCell::new(None),
@@ -406,14 +418,12 @@ mod tests {
     use super::super::pipeline::Pipeline;
     use super::*;
 
-    /// Extract the first job's `run_fn` from the pipeline, consume the
-    /// pipeline for its VM, build a minimal runtime, and return the
-    /// runtime handle plus the run_fn.
+    /// Consume the pipeline for its VM, build a minimal runtime,
+    /// and return the runtime and first job's run_fn.
     fn rt(source: &str, secrets: HashMap<String, SecretString>) -> (Rc<Runtime>, mlua::Function) {
         let pipeline = Pipeline::load(source, "ci.fnl").expect("load should succeed");
         let run_fn = pipeline.jobs()[0].run_fn.clone();
-        let fennel = pipeline.into_fennel();
-        let runtime = Rc::new(Runtime::for_test(fennel, secrets));
+        let runtime = Rc::new(Runtime::for_test(pipeline, secrets));
         let _ = RuntimeHandle(runtime.clone())
             .into_lua(runtime.lua())
             .expect("install runtime");
diff --git a/src/ci/pipeline.rs b/src/ci/pipeline.rs
index fb1bdac..5dbf7ab 100644
--- a/src/ci/pipeline.rs
+++ b/src/ci/pipeline.rs
@@ -101,13 +101,9 @@ impl Pipeline {
             .map(|&idx| &self.jobs[self.graph[idx]])
     }
 
-    /// Consume the pipeline and return its Fennel/Lua VM.
-    ///
-    /// The job functions (`run_fn`) are `'static` handles into this VM,
-    /// so they remain callable after extraction as long as the VM stays
-    /// alive.
-    pub(crate) fn into_fennel(self) -> Fennel {
-        self.fennel
+    /// Borrow the underlying Fennel/Lua VM.
+    pub(crate) fn fennel(&self) -> &Fennel {
+        &self.fennel
     }
 
     /// Return job IDs in topological order — dependencies before
diff --git a/src/ci/run.rs b/src/ci/run.rs
index ec5f75c..0d8fe77 100644
--- a/src/ci/run.rs
+++ b/src/ci/run.rs
@@ -217,12 +217,6 @@ pub struct Run {
     base: PathBuf,
     state: RunState,
     id: String,
-    /// Per-execution runtime shared with the Lua bridge: holds the
-    /// secrets exposed to the script, tracks the currently-running
-    /// job, and accumulates per-job captured `sh` output. Replaced
-    /// fresh each `execute` call so secrets are scoped to that call.
-    /// `None` before `execute` is called.
-    runtime: Option<Rc<Runtime>>,
 }
 
 impl Run {
@@ -247,12 +241,7 @@ impl Run {
     /// `pending/`, `active/`). Returns an error if `meta.yml` or
     /// `times.yml` are missing or unreadable.
     pub fn open(base: PathBuf, state: RunState, id: String) -> Result<Self> {
-        let run = Self {
-            base,
-            state,
-            id,
-            runtime: None,
-        };
+        let run = Self { base, state, id };
         run.read_meta()?;
         run.read_times()?;
         Ok(run)
@@ -265,9 +254,8 @@ impl Run {
     /// (`:quire/push` from `meta.yml`), and the per-job transitive-input
     /// sets; installs it on the VM, topo-sorts the jobs, transitions
     /// Pending → Active, then invokes each `run_fn` in dependency order
-    /// with the runtime handle as its sole argument. `(sh …)` calls
-    /// record their captured output under the current job — readable via
-    /// [`Run::outputs`] after `execute` returns. The run finishes in
+    /// with the runtime handle as its sole argument. Returns a map of
+    /// job id → captured `(sh …)` outputs. The run finishes in
     /// `Complete` if every job's `run_fn` returned without error,
     /// otherwise `Failed`.
     ///
@@ -277,29 +265,10 @@ impl Run {
         &mut self,
         pipeline: Pipeline,
         secrets: HashMap<String, SecretString>,
-    ) -> Result<()> {
+    ) -> Result<HashMap<String, Vec<ShOutput>>> {
         let meta = self.read_meta()?;
 
-        // Extract execution plan before consuming the pipeline for its VM.
-        let topo: Vec<String> = pipeline
-            .topo_order()
-            .into_iter()
-            .map(String::from)
-            .collect();
-        let transitive = pipeline.transitive_inputs();
-        let run_fns: Vec<(String, mlua::Function)> = topo
-            .iter()
-            .map(|id| {
-                let job = pipeline
-                    .job(id)
-                    .expect("topo_order returned a job id not in pipeline");
-                (job.id.clone(), job.run_fn.clone())
-            })
-            .collect();
-
-        let fennel = pipeline.into_fennel();
-        let runtime = Rc::new(Runtime::new(fennel, secrets, &meta, &transitive));
-        self.runtime = Some(runtime.clone());
+        let runtime = Rc::new(Runtime::new(pipeline, secrets, &meta));
 
         let lua = runtime.lua();
         let rt_value = RuntimeHandle(runtime.clone())
@@ -308,40 +277,31 @@ impl Run {
 
         self.transition(RunState::Active)?;
 
-        for (job_id, run_fn) in &run_fns {
-            self.runtime
-                .as_ref()
-                .expect("runtime installed")
-                .enter_job(job_id);
+        for job_id in runtime.topo_order() {
+            let run_fn = runtime
+                .job(job_id)
+                .expect("topo_order returned a job id not in pipeline")
+                .run_fn
+                .clone();
+
+            runtime.enter_job(job_id);
             let result = run_fn.call::<mlua::Value>(rt_value.clone());
-            self.runtime
-                .as_ref()
-                .expect("runtime installed")
-                .leave_job();
+            runtime.leave_job();
 
             if let Err(e) = result {
                 lua.remove_app_data::<Rc<Runtime>>();
                 self.transition(RunState::Failed)?;
                 return Err(Error::JobFailed {
-                    job: job_id.clone(),
+                    job: job_id.to_string(),
                     source: Box::new(e),
                 });
             }
         }
 
+        let outputs = runtime.take_outputs();
         lua.remove_app_data::<Rc<Runtime>>();
         self.transition(RunState::Complete)?;
-        Ok(())
-    }
-
-    /// Snapshot the `(sh …)` outputs recorded for `job_id` during the
-    /// most recent `execute` call. Empty if the job hasn't run or
-    /// produced no output.
-    pub fn outputs(&self, job_id: &str) -> Vec<ShOutput> {
-        self.runtime
-            .as_ref()
-            .map(|rt| rt.outputs(job_id))
-            .unwrap_or_default()
+        Ok(outputs)
     }
 
     /// Transition the run from its current state to a new state.
@@ -590,7 +550,6 @@ mod tests {
             base: PathBuf::from("/tmp/quire-test-runs/test.git"),
             state: RunState::Pending,
             id: uuid::Uuid::now_v7().to_string(),
-            runtime: None,
         };
 
         let result = run.transition(RunState::Active);
@@ -697,12 +656,12 @@ mod tests {
 (ci.job :b [:a] (fn [{: sh}] (sh ["echo" "from-b"])))"#,
         );
 
-        run.execute(pipeline, HashMap::new()).expect("execute");
+        let outputs = run.execute(pipeline, HashMap::new()).expect("execute");
 
         assert_eq!(run.state(), RunState::Complete);
 
-        let a = run.outputs("a");
-        let b = run.outputs("b");
+        let a = &outputs["a"];
+        let b = &outputs["b"];
         assert_eq!(a.len(), 1);
         assert_eq!(a[0].stdout, "from-a\n");
         assert_eq!(b.len(), 1);
@@ -750,10 +709,6 @@ mod tests {
             .expect_err("expected failure");
         assert!(matches!(err, Error::JobFailed { ref job, .. } if job == "a"));
         assert_eq!(run.state(), RunState::Failed);
-        assert!(
-            run.outputs("b").is_empty(),
-            "b should not have run after a failed"
-        );
     }
 
     #[test]
@@ -770,11 +725,11 @@ mod tests {
       (sh ["echo" push.sha push.ref]))))"#,
         );
 
-        run.execute(pipeline, HashMap::new()).expect("execute");
+        let outputs = run.execute(pipeline, HashMap::new()).expect("execute");
 
-        let outputs = run.outputs("grab");
-        assert_eq!(outputs.len(), 1);
-        assert_eq!(outputs[0].stdout, "abc123 refs/heads/main\n");
+        let grab = &outputs["grab"];
+        assert_eq!(grab.len(), 1);
+        assert_eq!(grab[0].stdout, "abc123 refs/heads/main\n");
     }
 
     #[test]
@@ -794,11 +749,11 @@ mod tests {
       (sh ["echo" push.sha]))))"#,
         );
 
-        run.execute(pipeline, HashMap::new()).expect("execute");
+        let outputs = run.execute(pipeline, HashMap::new()).expect("execute");
 
-        let outputs = run.outputs("b");
-        assert_eq!(outputs.len(), 1);
-        assert_eq!(outputs[0].stdout, "abc123\n");
+        let b = &outputs["b"];
+        assert_eq!(b.len(), 1);
+        assert_eq!(b[0].stdout, "abc123\n");
     }
 
     #[test]