Refactor CI module: rename types, collapse run_ref layers, drop redundant test
- Error::QuireCiExit → Error::ProcessFailed (non-zero exit is always a
failure; "exit" was ambiguous)
- Executor::QuireCi → Executor::Process (the variant describes the
mechanism, not the binary name)
- Collapse trigger_ref into run_ref_inner; run_ref now builds the
Transport then calls run_ref_inner inside the Sentry scope, removing
one indirection level and the confusingly swapped names
- Remove ci_source_reads_file_at_sha test (private-method test covered
by ci_pipeline_returns_pipeline_when_ci_fnl_present)
https://claude.ai/code/session_0135zw5K7qsn9thYkkpjX6HE
diff --git a/quire-server/src/ci/error.rs b/quire-server/src/ci/error.rs
index ca3238c..8da2215 100644
--- a/quire-server/src/ci/error.rs
+++ b/quire-server/src/ci/error.rs
@@ -58,7 +58,7 @@ pub enum Error {
},
#[error("quire-ci exited with status {exit:?}")]
- QuireCiExit { exit: Option<i32> },
+ ProcessFailed { exit: Option<i32> },
#[error("failed to parse quire-ci event stream at {}: {source}", path.display())]
EventStreamParse {
diff --git a/quire-server/src/ci/mod.rs b/quire-server/src/ci/mod.rs
index e043630..0d1f871 100644
--- a/quire-server/src/ci/mod.rs
+++ b/quire-server/src/ci/mod.rs
@@ -172,8 +172,7 @@ pub fn trigger(quire: &crate::Quire, event: &PushEvent) {
/// Set up Sentry trace scope and run CI for a single ref.
///
-/// Wraps `trigger_ref` with Sentry scope setup and error logging.
-/// `QuireCiExit` is logged at info (user-pipeline failure, visible in the
+/// `ProcessFailed` is logged at info (user-pipeline failure, visible in the
/// UI via run state); everything else is logged at error (operational).
#[allow(clippy::too_many_arguments)]
fn run_ref(
@@ -190,6 +189,7 @@ fn run_ref(
trace_id: sentry::protocol::TraceId,
span_id: sentry::protocol::SpanId,
) {
+ let transport = Transport::for_new_run(transport_mode, port);
sentry::with_scope(
|scope| {
scope.set_context(
@@ -203,18 +203,10 @@ fn run_ref(
);
},
|| {
- let transport = Transport::for_new_run(transport_mode, port);
- if let Err(e) = trigger_ref(
- repo,
- db_path,
- pushed_at,
- push_ref,
- secrets,
- executor,
- &transport,
- sentry,
+ if let Err(e) = run_ref_inner(
+ repo, db_path, pushed_at, push_ref, secrets, executor, &transport, sentry,
) {
- if matches!(e, error::Error::QuireCiExit { .. }) {
+ if matches!(e, error::Error::ProcessFailed { .. }) {
tracing::info!(
repo = %event_repo,
sha = %push_ref.new_sha, // cov-excl-line
@@ -236,7 +228,7 @@ fn run_ref(
/// Create and run CI for a single updated ref.
#[allow(clippy::too_many_arguments)]
-fn trigger_ref(
+fn run_ref_inner(
repo: &Repo,
db_path: &Path,
pushed_at: jiff::Timestamp,
@@ -270,7 +262,7 @@ fn trigger_ref(
let workspace = run.path().join("workspace");
run::materialize_workspace(&repo.path(), &push_ref.new_sha, &workspace)?;
match executor {
- Executor::QuireCi => {
+ Executor::Process => {
// Compilation happens inside quire-ci so a malformed ci.fnl is
// reported once, with the worker's trace context.
run.execute_via_quire_ci(&repo.path(), &workspace, &meta, secrets, sentry, transport)?;
@@ -421,20 +413,6 @@ mod tests {
assert!(result.is_err(), "bad Fennel should fail");
}
- #[test]
- fn ci_source_reads_file_at_sha() {
- let source = "(local ci (require :quire.ci))\n(ci.job :x [:quire/push] (fn [] nil))";
- let (_dir, quire, name) = bare_repo_with_ci(source);
- let repo = quire.repo(&name).expect("repo");
- let ci = repo.ci();
- let sha = head_sha(&repo);
- let content = ci
- .source(&sha)
- .expect("source should succeed")
- .expect("should have content");
- assert!(content.contains(":x"));
- }
-
/// Serialize PATH mutations so concurrent tests don't observe each
/// other's fake binaries.
static PATH_MUTEX: std::sync::Mutex<()> = std::sync::Mutex::new(());
@@ -503,13 +481,13 @@ mod tests {
let (_fake_dir, fake_path) = fake_quire_ci(0);
let trigger_result = with_path(&fake_path, || {
- trigger_ref(
+ run_ref_inner(
&repo,
&quire.db_path(),
pushed_at,
&push_ref,
&HashMap::new(),
- Executor::QuireCi,
+ Executor::Process,
&Transport::Filesystem,
None,
)
@@ -558,13 +536,13 @@ mod tests {
let (_fake_dir, fake_path) = fake_quire_ci(1);
let trigger_result = with_path(&fake_path, || {
- trigger_ref(
+ run_ref_inner(
&repo,
&quire.db_path(),
pushed_at,
&push_ref,
&HashMap::new(),
- Executor::QuireCi,
+ Executor::Process,
&Transport::Filesystem,
None,
)
@@ -602,13 +580,13 @@ mod tests {
r#ref: "refs/heads/main".to_string(),
};
- trigger_ref(
+ run_ref_inner(
&repo,
&quire.db_path(),
pushed_at,
&push_ref,
&HashMap::new(),
- Executor::QuireCi,
+ Executor::Process,
&Transport::Filesystem,
None,
)
diff --git a/quire-server/src/ci/run.rs b/quire-server/src/ci/run.rs
index f9736c0..7af7ed3 100644
--- a/quire-server/src/ci/run.rs
+++ b/quire-server/src/ci/run.rs
@@ -19,7 +19,7 @@ pub use quire_core::ci::run::RunMeta;
/// How a run dispatches its pipeline.
///
-/// `QuireCi` shells out to the `quire-ci` binary, which compiles and
+/// `Process` shells out to the `quire-ci` binary, which compiles and
/// runs the pipeline in a separate process. The enum is kept open
/// so a future `Docker` executor can be added without another config
/// migration.
@@ -27,7 +27,7 @@ pub use quire_core::ci::run::RunMeta;
#[serde(rename_all = "kebab-case")]
pub enum Executor {
#[default]
- QuireCi,
+ Process,
}
/// Transport for CI ↔ server communication.
@@ -411,7 +411,7 @@ impl Run {
if !status.success() {
self.transition(RunState::Failed, Some("quire-ci-exit"))?;
- return Err(Error::QuireCiExit {
+ return Err(Error::ProcessFailed {
exit: status.code(),
});
}
diff --git a/quire-server/src/quire/mod.rs b/quire-server/src/quire/mod.rs
index 7a050c2..1d88b54 100644
--- a/quire-server/src/quire/mod.rs
+++ b/quire-server/src/quire/mod.rs
@@ -40,7 +40,7 @@ fn default_port() -> u16 {
#[derive(serde::Deserialize, Debug, Default)]
pub struct CiConfig {
/// How the orchestrator dispatches CI runs. Defaults to shelling
- /// out to the `quire-ci` binary via `Executor::QuireCi`.
+ /// out to the `quire-ci` binary via `Executor::Process`.
#[serde(default)]
pub executor: Executor,
/// Transport for CI ↔ server communication.
@@ -420,7 +420,7 @@ mod tests {
let q = Quire::new(dir.path().to_path_buf());
let config = q.global_config().expect("global_config should load");
assert_eq!(config.ci.transport, TransportMode::Filesystem);
- assert_eq!(config.ci.executor, Executor::QuireCi);
+ assert_eq!(config.ci.executor, Executor::Process);
assert_eq!(config.port, 3000);
}