Cycle members were also failing the reachability check, generating
Cycle + N Unreachable errors for one bad cycle. Skip them.

(ci.sh ...) reported "argv list is empty" for any zero-len table,
including string-keyed maps passed by mistake. One accurate message
handles both shapes.

Assisted-by: Claude Opus 4.7 (1M context) via Claude Code

diff --git a/src/ci/lua.rs b/src/ci/lua.rs
index 410f412..15148c6 100644
--- a/src/ci/lua.rs
+++ b/src/ci/lua.rs
@@ -146,6 +146,10 @@ impl Cmd {
     /// Spawn this command with the given options, blocking until exit,
     /// and capture the result. Inherits the runner's env with
     /// `opts.env` merged on top.
+    //
+    // TODO: stream stdout/stderr live instead of buffering. `output()`
+    // captures the full child output in memory and only returns at exit,
+    // so long-running or chatty jobs show nothing until they finish.
     fn run(self, opts: ShOpts) -> std::io::Result<Output> {
         let mut command: std::process::Command = self.into();
         for (k, v) in opts.env {
@@ -155,6 +159,8 @@ impl Cmd {
             command.current_dir(cwd);
         }
         let output = command.output()?;
+        // Signal-killed processes have no exit code; collapse them to -1
+        // for now. Surfacing the signal as a separate field is future work.
         Ok(Output {
             exit: output.status.code().unwrap_or(-1),
             stdout: String::from_utf8_lossy(&output.stdout).into_owned(),
@@ -171,10 +177,16 @@ impl mlua::FromLua for Cmd {
         match &value {
             mlua::Value::String(_) => lua.from_value(value),
             mlua::Value::Table(t) if t.raw_len() == 0 => {
+                // `raw_len() == 0` covers both an empty sequence (`[]`)
+                // and a string-keyed table (`{:env {...}}`) passed in
+                // place of an argv list. One message handles both.
                 Err(mlua::Error::FromLuaConversionError {
                     from: "table",
                     to: "Cmd".into(),
-                    message: Some("ci.sh: argv list is empty".into()),
+                    message: Some(
+                        "ci.sh: cmd must be a non-empty sequence of strings or a shell string"
+                            .into(),
+                    ),
                 })
             }
             mlua::Value::Table(_) => lua.from_value(value),
@@ -371,6 +383,29 @@ mod tests {
         );
     }
 
+    #[test]
+    fn ci_sh_rejects_non_sequence_table_as_cmd() {
+        let f = fennel();
+        let pipeline = load(
+            &f,
+            r#"(local ci (require :quire.ci))
+(ci.job :go [:quire/push] (fn [_] (ci.sh {:env {:FOO "bar"}})))"#,
+            "ci.fnl",
+            "ci.fnl",
+            HashMap::new(),
+        )
+        .expect("load should succeed");
+        let err = pipeline.jobs()[0]
+            .run_fn
+            .call::<mlua::Value>(())
+            .unwrap_err();
+        let msg = err.to_string();
+        assert!(
+            msg.contains("sequence"),
+            "expected sequence-shape error, got: {msg}"
+        );
+    }
+
     #[test]
     fn ci_sh_rejects_empty_argv() {
         let f = fennel();
diff --git a/src/ci/mod.rs b/src/ci/mod.rs
index 9bc0d0e..629b714 100644
--- a/src/ci/mod.rs
+++ b/src/ci/mod.rs
@@ -1,8 +1,8 @@
 //! CI: trigger runs from push events, validate the job graph.
 
 mod lua;
-pub mod pipeline;
-pub mod run;
+mod pipeline;
+mod run;
 
 pub use pipeline::{Job, LoadError, Pipeline, ValidationError};
 pub use run::{Run, RunMeta, RunState, RunTimes, Runs};
diff --git a/src/ci/pipeline.rs b/src/ci/pipeline.rs
index 0626ea9..2ba4a97 100644
--- a/src/ci/pipeline.rs
+++ b/src/ci/pipeline.rs
@@ -201,6 +201,7 @@ pub struct LoadError {
 /// here.
 fn validate_post_graph(jobs: &[Job]) -> std::result::Result<(), Vec<ValidationError>> {
     let mut errors = Vec::new();
+    let mut cycle_members: std::collections::HashSet<&str> = std::collections::HashSet::new();
 
     // Rule 1: acyclic.
     //
@@ -236,15 +237,27 @@ fn validate_post_graph(jobs: &[Job]) -> std::result::Result<(), Vec<ValidationEr
             .filter_map(|&idx| jobs.iter().find(|j| j.id == graph[idx]))
             .collect();
         members.sort_by(|a, b| a.id.cmp(&b.id));
+        for j in &members {
+            cycle_members.insert(j.id.as_str());
+        }
         let cycle_jobs = members.iter().map(|j| j.id.clone()).collect();
         let spans = members.iter().map(|j| j.span).collect();
         errors.push(ValidationError::Cycle { cycle_jobs, spans });
     }
 
     // Rule 3: reachability — every job's transitive inputs must include a source ref.
+    //
+    // TODO: replace the `quire/` prefix check with a whitelist of real
+    // source refs (`quire/push`, etc.) once those are implemented, so
+    // typos like `:quire/posh` don't silently make a job "reachable."
     let is_source = |name: &str| name.starts_with("quire/");
 
     for job in jobs {
+        // Cycle members are already reported via Cycle; skip them here so
+        // a single bad cycle doesn't generate N+1 errors.
+        if cycle_members.contains(job.id.as_str()) {
+            continue;
+        }
         let mut visited = std::collections::HashSet::new();
         let mut stack: Vec<&str> = job.inputs.iter().map(|s| s.as_str()).collect();
         let mut found_source = false;
@@ -462,13 +475,36 @@ mod tests {
         );
     }
 
+    #[test]
+    fn validate_does_not_double_report_cycle_as_unreachable() {
+        // Jobs in a cycle are technically also unreachable from any
+        // source ref, but reporting both is noise. Cycle alone is enough.
+        let jobs = parsed_jobs(
+            r#"
+(local ci (require :quire.ci))
+(ci.job :a [:b] (fn [_] nil))
+(ci.job :b [:a] (fn [_] nil))
+"#,
+        );
+        let errs = validate_post_graph(&jobs).unwrap_err();
+        let unreachable_count = errs
+            .iter()
+            .filter(|e| matches!(e, ValidationError::Unreachable { .. }))
+            .count();
+        assert_eq!(
+            unreachable_count, 0,
+            "cycle members should not also be reported as unreachable: {errs:?}"
+        );
+    }
+
     #[test]
     fn validate_rejects_unreachable_jobs() {
-        // An orphan with non-empty self-input passes pre-graph rules
-        // and reaches the post-graph reachability check.
+        // A job whose only input names a non-existent job passes
+        // pre-graph rules (inputs is non-empty, id has no slash) and
+        // reaches the post-graph reachability check.
         let jobs = parsed_jobs(
             r#"(local ci (require :quire.ci))
-(ci.job :orphan [:orphan] (fn [_] nil))"#,
+(ci.job :orphan [:does-not-exist] (fn [_] nil))"#,
         );
         let errs = validate_post_graph(&jobs).unwrap_err();
         assert!(