Extract CI errors into ci::error, introduce secret::Error
Give SecretString::reveal() its own error type (secret::Error) so
both crate::Error and ci::Error can impl From<secret::Error> without
manual translation at call sites.

Move CI-specific error variants (Pipeline, InvalidTransition,
JobFailed, DockerUnavailable, DockerfileMissing, etc.) from the
top-level Error into ci::error::Error. The top-level Error wraps it
via Error::Ci. Drop crate::Error::Git and crate::Error::SecretResolve
which had no remaining call sites outside ci/.

Assisted-by: GLM-5.1 via pi
change lvlvtymzunmxovmotqsmrlzpwmoxnsvz
commit 688080edaa8a0cfc7c06073014c03ce8d8ddf956
author Alpha Chen <alpha@kejadlen.dev>
date
parent orpvlnxo
diff --git a/src/ci/docker.rs b/src/ci/docker.rs
index 8d6bad4..e23d34a 100644
--- a/src/ci/docker.rs
+++ b/src/ci/docker.rs
@@ -2,7 +2,7 @@
 
 use std::path::Path;
 
-use crate::{Error, Result};
+use super::error::{Error, Result};
 
 /// Returns true iff the docker daemon is reachable. Used to gate
 /// integration tests; calls `docker info` and treats any failure
diff --git a/src/ci/error.rs b/src/ci/error.rs
new file mode 100644
index 0000000..c1f9124
--- /dev/null
+++ b/src/ci/error.rs
@@ -0,0 +1,95 @@
+//! CI error types.
+
+use miette::Diagnostic;
+
+use super::pipeline::PipelineError;
+use super::run::RunState;
+use crate::fennel::FennelError;
+use crate::secret;
+
+/// Errors produced by CI operations.
+#[derive(Debug, thiserror::Error, Diagnostic)]
+pub enum Error {
+    #[error("io error: {0}")]
+    Io(#[from] std::io::Error),
+
+    #[error(transparent)]
+    #[diagnostic(transparent)]
+    Fennel(#[from] Box<FennelError>),
+
+    #[error(transparent)]
+    #[diagnostic(transparent)]
+    Pipeline(Box<PipelineError>),
+
+    #[error("invalid run transition: {from:?} -> {to:?}")]
+    InvalidTransition { from: RunState, to: RunState },
+
+    #[error(transparent)]
+    Lua(Box<mlua::Error>),
+
+    #[error("job '{job}' failed")]
+    JobFailed {
+        job: String,
+        #[source]
+        source: Box<Error>,
+    },
+
+    #[error("docker is not available — install docker and ensure the daemon is running")]
+    DockerUnavailable,
+
+    #[error("missing .quire/Dockerfile")]
+    DockerfileMissing,
+
+    #[error("workspace materialization failed")]
+    WorkspaceMaterializationFailed {
+        #[source]
+        source: std::io::Error,
+    },
+
+    #[error("image build failed")]
+    ImageBuildFailed {
+        #[source]
+        source: std::io::Error,
+    },
+
+    #[error("container start failed")]
+    ContainerStartFailed {
+        #[source]
+        source: std::io::Error,
+    },
+
+    #[error("git error: {0}")]
+    Git(String),
+
+    #[error(transparent)]
+    Yaml(#[from] serde_yaml_ng::Error),
+
+    #[error(transparent)]
+    Utf8(#[from] std::string::FromUtf8Error),
+
+    #[error(transparent)]
+    Secret(#[from] secret::Error),
+
+    #[error("unknown secret: {0:?}")]
+    UnknownSecret(String),
+}
+
+pub type Result<T> = std::result::Result<T, Error>;
+
+impl From<PipelineError> for Error {
+    fn from(err: PipelineError) -> Self {
+        Error::Pipeline(Box::new(err))
+    }
+}
+
+impl From<FennelError> for Error {
+    fn from(err: FennelError) -> Self {
+        Error::Fennel(Box::new(err))
+    }
+}
+
+impl From<mlua::Error> for Error {
+    fn from(err: mlua::Error) -> Self {
+        Error::Lua(Box::new(err))
+    }
+}
diff --git a/src/ci/mirror.rs b/src/ci/mirror.rs
index 395c6c7..a2d2cc6 100644
--- a/src/ci/mirror.rs
+++ b/src/ci/mirror.rs
@@ -12,11 +12,10 @@ use std::rc::Rc;
 
 use mlua::{Lua, LuaSerdeExt};
 
+use super::error::{Error, Result};
 use super::pipeline::{self, DefinitionError, Job, RunFn};
 use super::registration::Registration;
 use super::runtime::{Cmd, Runtime, ShOpts};
-use crate::Result;
-use crate::error::Error;
 
 /// Closure state for the `quire/mirror` job's run-fn: everything the
 /// tag-and-push needs at execute time, captured once at registration.
@@ -338,7 +337,7 @@ mod tests {
         let Err(err) = compile(source, "ci.fnl") else {
             panic!("expected error");
         };
-        let crate::Error::Pipeline(pe) = err else {
+        let crate::ci::error::Error::Pipeline(pe) = err else {
             panic!("expected PipelineError, got {err:?}");
         };
         assert!(
@@ -357,7 +356,7 @@ mod tests {
         let Err(err) = compile(source, "ci.fnl") else {
             panic!("expected error");
         };
-        let crate::Error::Pipeline(pe) = err else {
+        let crate::ci::error::Error::Pipeline(pe) = err else {
             panic!("expected PipelineError, got {err:?}");
         };
         pe.diagnostics
diff --git a/src/ci/mod.rs b/src/ci/mod.rs
index a67267a..9fe2d73 100644
--- a/src/ci/mod.rs
+++ b/src/ci/mod.rs
@@ -9,6 +9,9 @@ mod registration;
 mod run;
 mod runtime;
 
+pub(crate) mod error;
+
+pub use error::{Error, Result};
 pub use pipeline::{DefinitionError, Diagnostic, Job, Pipeline, PipelineError, StructureError};
 pub use run::{Executor, Run, RunMeta, RunState, RunTimes, Runs, materialize_workspace};
 
@@ -25,7 +28,6 @@ pub struct CommitRef {
 
 use std::path::PathBuf;
 
-use crate::Result;
 use crate::display_chain;
 use crate::event::{PushEvent, PushRef};
 use crate::quire::Repo;
@@ -62,7 +64,7 @@ impl Ci {
     /// Returns `Ok(None)` if the repo has no ci.fnl at that commit.
     /// Errors if the Fennel source fails to parse/evaluate or if the
     /// resulting job graph violates any structural rule.
-    pub fn pipeline(&self, commit: &CommitRef) -> Result<Option<Pipeline>> {
+    pub fn pipeline(&self, commit: &CommitRef) -> error::Result<Option<Pipeline>> {
         let Some(source) = self.source(&commit.sha)? else {
             return Ok(None);
         };
@@ -75,7 +77,7 @@ impl Ci {
     ///
     /// Returns `Ok(None)` if the file does not exist at that commit,
     /// `Ok(Some(contents))` if it does, or `Err` for unexpected failures.
-    fn source(&self, sha: &str) -> Result<Option<String>> {
+    fn source(&self, sha: &str) -> error::Result<Option<String>> {
         let output = self
             .git(&["show", &format!("{sha}:{CI_FNL}")])
             .stdout(std::process::Stdio::piped())
@@ -87,7 +89,7 @@ impl Ci {
             if stderr.contains("does not exist") || stderr.contains("not found") {
                 return Ok(None);
             }
-            return Err(crate::Error::Git(format!(
+            return Err(error::Error::Git(format!(
                 "failed to read {CI_FNL} at {sha}: {stderr}"
             )));
         }
@@ -144,7 +146,7 @@ fn trigger_ref(
     pushed_at: jiff::Timestamp,
     push_ref: &PushRef,
     secrets: &HashMap<String, crate::secret::SecretString>,
-) -> Result<()> {
+) -> error::Result<()> {
     let ci = repo.ci();
 
     let Some(source) = ci.source(&push_ref.new_sha)? else {
diff --git a/src/ci/pipeline.rs b/src/ci/pipeline.rs
index b715f7a..e914fe4 100644
--- a/src/ci/pipeline.rs
+++ b/src/ci/pipeline.rs
@@ -11,8 +11,8 @@ use petgraph::Graph;
 use petgraph::graph::NodeIndex;
 use petgraph::visit::{Bfs, Reversed};
 
+use super::error::Result;
 use super::registration::{self, Registrations};
-use crate::Result;
 use crate::fennel::Fennel;
 
 /// A registration-time error caught while individual `(ci.job …)` and
@@ -515,7 +515,7 @@ mod tests {
         let f = Fennel::new().expect("Fennel::new() should succeed");
         let err =
             registration::register(&f, source, "ci.fnl").expect_err("expected registration errors");
-        let crate::Error::Pipeline(pe) = err else {
+        let crate::ci::error::Error::Pipeline(pe) = err else {
             panic!("expected PipelineError, got {err:?}")
         };
         pe.diagnostics
@@ -808,7 +808,7 @@ mod tests {
 (ci.job :orphan [:does-not-exist] (fn [_] nil))"#,
             "ci.fnl",
         );
-        let Err(crate::Error::Pipeline(pe)) = result else {
+        let Err(crate::ci::error::Error::Pipeline(pe)) = result else {
             panic!("expected PipelineError")
         };
         for d in &pe.diagnostics {
diff --git a/src/ci/registration.rs b/src/ci/registration.rs
index e76fb12..b452db1 100644
--- a/src/ci/registration.rs
+++ b/src/ci/registration.rs
@@ -14,9 +14,9 @@ use mlua::{IntoLua, Lua};
 
 use miette::NamedSource;
 
+use super::error::Result;
 use super::mirror;
 use super::pipeline::{self, DefinitionError, Diagnostic, Job, PipelineError, RunFn};
-use crate::Result;
 use crate::fennel::Fennel;
 
 /// Output of [`register`]: jobs and image successfully registered
diff --git a/src/ci/run.rs b/src/ci/run.rs
index a3bf7a1..e48e5f6 100644
--- a/src/ci/run.rs
+++ b/src/ci/run.rs
@@ -12,11 +12,11 @@ use std::rc::Rc;
 use jiff::Timestamp;
 use mlua::IntoLua;
 
+use super::error::{Error, Result};
 use super::pipeline::{Pipeline, RunFn};
 use super::runtime::{ExecutorRuntime, Runtime, RuntimeHandle, ShOutput};
 use crate::display_chain;
 use crate::secret::SecretString;
-use crate::{Error, Result};
 
 /// The execution mode for a run. Host runs `sh` directly on the host.
 /// Docker materializes a container and routes `sh` through `docker exec`.
@@ -1611,7 +1611,7 @@ mod tests {
         );
 
         pipeline.replace_first_run_fn(RunFn::Rust(Rc::new(|_rt| {
-            Err(crate::Error::Git("simulated rust failure".into()))
+            Err(Error::Git("simulated rust failure".into()))
         })));
 
         let err = run
diff --git a/src/ci/runtime.rs b/src/ci/runtime.rs
index 597ecae..dca1282 100644
--- a/src/ci/runtime.rs
+++ b/src/ci/runtime.rs
@@ -180,11 +180,11 @@ impl Runtime {
 
     /// Resolve a declared secret by name. Errors if the name isn't
     /// declared or the secret's source can't be read.
-    pub(super) fn secret(&self, name: &str) -> crate::Result<String> {
+    pub(super) fn secret(&self, name: &str) -> super::error::Result<String> {
         let secret = self
             .secrets
             .get(name)
-            .ok_or_else(|| crate::Error::UnknownSecret(name.to_string()))?;
+            .ok_or_else(|| super::error::Error::UnknownSecret(name.to_string()))?;
         Ok(secret.reveal()?.to_string())
     }
 
@@ -196,7 +196,7 @@ impl Runtime {
     /// the per-run container; the bind-mounted workspace is the
     /// container's working directory, and `opts.env` is forwarded as
     /// `-e KEY=VAL` flags.
-    pub(super) fn sh(&self, cmd: Cmd, opts: ShOpts) -> crate::Result<ShOutput> {
+    pub(super) fn sh(&self, cmd: Cmd, opts: ShOpts) -> super::error::Result<ShOutput> {
         let output = match self.docker_target() {
             None => cmd.run(opts, &self.workspace)?,
             Some((container_id, work_dir)) => {
diff --git a/src/error.rs b/src/error.rs
index 6a2e703..b469ed9 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -1,21 +1,14 @@
 use miette::Diagnostic;
 
-use crate::ci::{PipelineError, RunState};
+use crate::ci::Error as CiError;
 use crate::fennel::FennelError;
+use crate::secret;
 
 #[derive(Debug, thiserror::Error, Diagnostic)]
 pub enum Error {
     #[error("io error: {0}")]
     Io(#[from] std::io::Error),
 
-    // Stored as a string because `OnceLock` in `SecretString::reveal` caches
-    // the error and `std::io::Error` is not `Clone`. See `secret.rs` for details.
-    #[error("secret resolution failed: {0}")]
-    SecretResolve(String),
-
-    #[error("unknown secret: {0:?}")]
-    UnknownSecret(String),
-
     #[error("config not found: {0}")]
     ConfigNotFound(String),
 
@@ -25,47 +18,10 @@ pub enum Error {
 
     #[error(transparent)]
     #[diagnostic(transparent)]
-    Pipeline(Box<PipelineError>),
-
-    #[error("invalid run transition: {from:?} -> {to:?}")]
-    InvalidTransition { from: RunState, to: RunState },
+    Ci(#[from] CiError),
 
     #[error(transparent)]
-    Lua(Box<mlua::Error>),
-
-    #[error("job '{job}' failed")]
-    JobFailed {
-        job: String,
-        #[source]
-        source: Box<Error>,
-    },
-
-    #[error("docker is not available — install docker and ensure the daemon is running")]
-    DockerUnavailable,
-
-    #[error("missing .quire/Dockerfile")]
-    DockerfileMissing,
-
-    #[error("workspace materialization failed")]
-    WorkspaceMaterializationFailed {
-        #[source]
-        source: std::io::Error,
-    },
-
-    #[error("image build failed")]
-    ImageBuildFailed {
-        #[source]
-        source: std::io::Error,
-    },
-
-    #[error("container start failed")]
-    ContainerStartFailed {
-        #[source]
-        source: std::io::Error,
-    },
-
-    #[error("git error: {0}")]
-    Git(String),
+    Secret(#[from] secret::Error),
 
     #[error(transparent)]
     Yaml(#[from] serde_yaml_ng::Error),
@@ -115,18 +71,6 @@ impl From<FennelError> for Error {
     }
 }
 
-impl From<PipelineError> for Error {
-    fn from(err: PipelineError) -> Self {
-        Error::Pipeline(Box::new(err))
-    }
-}
-
-impl From<mlua::Error> for Error {
-    fn from(err: mlua::Error) -> Self {
-        Error::Lua(Box::new(err))
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -165,14 +109,17 @@ mod tests {
 
     #[test]
     fn display_chain_handles_no_source() {
-        let err = Error::Git("boom".to_string());
-        assert_eq!(display_chain(&err).to_string(), "git error: boom");
+        let err = Error::ConfigNotFound("missing.yml".to_string());
+        assert_eq!(
+            display_chain(&err).to_string(),
+            "config not found: missing.yml"
+        );
     }
 
     #[test]
     fn from_pipeline_error() {
         let source = "(ci.job :a [] (fn [_] nil))";
-        let pipeline_err = PipelineError {
+        let pipeline_err = crate::ci::PipelineError {
             src: miette::NamedSource::new("ci.fnl", source.to_string()),
             diagnostics: vec![crate::ci::Diagnostic::Definition(
                 crate::ci::DefinitionError::EmptyInputs {
@@ -181,7 +128,8 @@ mod tests {
                 },
             )],
         };
-        let err: Error = pipeline_err.into();
+        let ci_err = crate::ci::Error::from(pipeline_err);
+        let err: Error = ci_err.into();
         assert!(err.to_string().contains("ci.fnl has errors"));
     }
 }
diff --git a/src/secret.rs b/src/secret.rs
index 2d54d5a..d162d0c 100644
--- a/src/secret.rs
+++ b/src/secret.rs
@@ -1,11 +1,24 @@
 use std::path::PathBuf;
 use std::sync::OnceLock;
 
-use crate::{Error, Result};
-
 #[cfg(test)]
 use crate::fennel::Fennel;
 
+/// Errors produced by secret resolution.
+#[derive(Debug, Clone, thiserror::Error)]
+pub enum Error {
+    /// File-backed secret could not be read.
+    ///
+    /// Stored as a string because `OnceLock` in `SecretString::reveal` caches
+    /// the error and `std::io::Error` is not `Clone`. Once `once_cell_try`
+    /// stabilizes (allowing `OnceLock::get_or_try_init` with a separate error
+    /// type), we can store a structured error instead of a string.
+    #[error("secret resolution failed: {0}")]
+    Resolve(String),
+}
+
+pub type Result<T> = std::result::Result<T, Error>;
+
 /// A string value that deserializes from either a plain literal or a file path.
 ///
 /// Fennel config can provide a secret as:
@@ -63,7 +76,7 @@ impl SecretString {
                 })
                 .as_ref()
                 .map(|s| s.as_str())
-                .map_err(|msg| Error::SecretResolve(msg.clone())),
+                .map_err(|msg| Error::Resolve(msg.clone())),
         }
     }
 }
@@ -185,8 +198,8 @@ mod tests {
         let secret = SecretString::from_file(PathBuf::from("/no/such/file/ever").as_path());
         let err = secret.reveal().unwrap_err();
         assert!(
-            matches!(err, Error::SecretResolve(_)),
-            "expected SecretResolve error, got {err:?}"
+            matches!(err, Error::Resolve(_)),
+            "expected Resolve error, got {err:?}"
         );
     }