refactor: drop dead columns from runs table (migration 0007)
Remove eight columns that carried no live data with the Process executor: container_id, workspace_path, image_tag, build_started_at_ms, build_finished_at_ms, container_started_at_ms, container_stopped_at_ms, and sentry_trace_id. Since container_id appeared in CHECK constraints, migration 0007 recreates the runs table rather than using ALTER TABLE DROP COLUMN. The CHECK constraint is simplified to drop the container_id IS NULL clauses. Code changes: - Runs::create: remove workspace_path from INSERT (path still created on disk) - supersede_existing: remove docker kill logic; query SELECT id only - reconcile_orphans, transition: remove container_id = NULL from UPDATEs - Test helpers in handlers.rs and dev.rs: remove dead columns from INSERTs https://claude.ai/code/session_01SpFto4X7xN2xnqgHbP62eB
diff --git a/docs/CI-STATE.md b/docs/CI-STATE.md
index 1b03731..bf448a3 100644
--- a/docs/CI-STATE.md
+++ b/docs/CI-STATE.md
@@ -57,7 +57,7 @@ stateDiagram-v2
| `active → complete` | `Run::transition`, called from `Run::execute` | `quire-ci` exited 0 and `RunFinished { outcome: Success }` was ingested. Stamps `finished_at_ms`, clears `container_id`. | — |
| `active → failed` | `Run::execute` | `quire-ci` exited 0 and `RunFinished { outcome: PipelineFailure }` was ingested — a job's run-fn returned an error. | `"pipeline-failure"` |
| `active → failed` | `Run::execute` | `quire-ci` exited non-zero, or exited 0 but emitted no `RunFinished` event (process crash or panic). | `"process-crashed"` |
-| `{pending, active} → superseded` | `Runs::supersede_existing` (`run.rs:155`) via raw SQL, **bypassing `transition`** | A new `Runs::create` for the same `(repo, ref)` arrived. Pending rows are flipped directly; active rows have their container killed (`docker kill`) first. | — |
+| `{pending, active} → superseded` | `Runs::supersede_existing` (`run.rs:155`) via raw SQL, **bypassing `transition`** | A new `Runs::create` for the same `(repo, ref)` arrived. Both pending and active rows are flipped directly. | — |
| `{pending, active} → failed` | `reconcile_orphans` (`run.rs:205`) via raw SQL, **bypassing `transition`** | Startup-time cleanup of rows left behind by a previous `quire serve` instance. | `"orphaned"` |
`Run::transition(to, failure_kind)`'s allowed-transition match:
@@ -73,15 +73,15 @@ In practice only `(Active, Complete)` and `(Active, Failed)` are exercised via `
### Database invariants
-The DB enforces shape per state via a `CHECK` constraint in `migrations/0001_initial.sql`:
+The DB enforces shape per state via a `CHECK` constraint (see `migrations/0007_schema_cleanup.sql`):
-| State | `started_at_ms` | `finished_at_ms` | `container_id` |
-| --- | --- | --- | --- |
-| `pending` | NULL | NULL | NULL |
-| `active` | set | NULL | (any) |
-| `complete` | set | set | NULL |
-| `failed` | (any) | set | NULL |
-| `superseded` | (any) | set | NULL |
+| State | `started_at_ms` | `finished_at_ms` |
+| --- | --- | --- |
+| `pending` | NULL | NULL |
+| `active` | set | NULL |
+| `complete` | set | set |
+| `failed` | (any) | set |
+| `superseded` | (any) | set |
Plus monotonicity: `started_at_ms >= queued_at_ms`, `finished_at_ms >= started_at_ms`. `started_at_ms`, `finished_at_ms`, and `failure_kind` are stamped at most once each, via `COALESCE` in the `UPDATE`.
@@ -225,49 +225,39 @@ sequenceDiagram
Two things in `CI.md` that the code does *not* yet implement at this layer:
* **Queue + Notify wakeup.** `CI.md` describes a separate runner task pulled from a SQLite queue via `tokio::sync::Notify`. Today `ci::trigger` is called **synchronously** on the listener's tokio task — one push at a time, no queue, no separate runner. Max-concurrency-1 falls out of this trivially, but it isn't the architecture in `CI.md`.
-* **Per-run container.** `CI.md` says `docker run` at run start, `docker exec` per `(sh …)`, `docker stop` at end. `quire-ci` invokes `(sh …)` directly on the host process; the `container_id` column is unused by the current executor and read only by `supersede_existing` (which calls `docker kill` if it's set).
+* **Per-run container.** `CI.md` says `docker run` at run start, `docker exec` per `(sh …)`, `docker stop` at end. `quire-ci` invokes `(sh …)` directly on the host process. The Docker-executor schema columns (`container_id`, `image_tag`, build/container timestamps) have been removed in migration 0007.
## Schema column inventory
-A column-by-column map of what's live vs. dead weight with the current Process executor. "Dead" means no code path writes a non-NULL value **and** no code path reads it back, or writes it but nothing reads it.
-
### `runs` table
-| Column | Written by | Read by | Status |
-| --- | --- | --- | --- |
-| `id` | `Runs::create` | everywhere | **live** |
-| `repo` | `Runs::create` | `supersede_existing`, web handlers | **live** |
-| `ref_name` | `Runs::create` | `supersede_existing`, web handlers, bootstrap response | **live** |
-| `sha` | `Runs::create` | `read_meta`, bootstrap response, web handlers | **live** |
-| `pushed_at_ms` | `Runs::create` | `read_meta`, web handlers | **live** |
-| `state` | `Runs::create` (→ `pending`) + every transition | everywhere | **live** |
-| `failure_kind` | `Run::transition(Failed, …)`, `reconcile_orphans` | web handlers | **live** |
-| `queued_at_ms` | `Runs::create` | web handlers | **live** |
-| `started_at_ms` | `transition(Active)`, also stamped as fallback in `Complete/Failed/Superseded` | `read_started_at`, web handlers | **live** |
-| `finished_at_ms` | `transition(Complete/Failed/Superseded)` | `read_finished_at`, web handlers | **live** |
-| `run_token` | `Runs::create` (API sessions only) | `verify_run_token` middleware | **live** |
-| `git_dir` | `Run::store_bootstrap_data` (API sessions only) | bootstrap endpoint | **live** |
-| `traceparent` | `Run::store_bootstrap_data` (API sessions only) | bootstrap endpoint | **live** |
-| `container_id` | nothing (always inserted/cleared as `NULL`) | `supersede_existing` checks it before calling `docker kill`, then clears it on supersede | **dead** — always NULL with the Process executor; the read-and-clear is a hook for a future Docker executor that never shipped |
-| `workspace_path` | `Runs::create` (hardcoded in test helpers too) | nothing reads it back | **dead** — the runtime reconstructs the path as `<base_dir>/<run_id>/workspace` from config + ID |
-| `image_tag` | nothing (always `NULL`) | nothing | **dead** — Docker executor placeholder |
-| `build_started_at_ms` | nothing (always `NULL`) | nothing | **dead** — Docker executor placeholder |
-| `build_finished_at_ms` | nothing (always `NULL`) | nothing | **dead** — Docker executor placeholder |
-| `container_started_at_ms` | nothing (always `NULL`) | nothing | **dead** — Docker executor placeholder |
-| `container_stopped_at_ms` | nothing (always `NULL`) | nothing | **dead** — Docker executor placeholder |
-| `sentry_trace_id` | nothing (added in migration 0004, never written) | nothing | **dead** — superseded by `traceparent` (migration 0006) before the column was ever used |
-
-The five `image_tag` / `build_*` / `container_{started,stopped}_at_ms` columns and `sentry_trace_id` are pure schema debt — they were added speculatively for a Docker executor that was never implemented. `container_id` and `workspace_path` are written but never read back by live code paths, so they are candidates for removal as well.
+| Column | Written by | Read by |
+| --- | --- | --- |
+| `id` | `Runs::create` | everywhere |
+| `repo` | `Runs::create` | `supersede_existing`, web handlers |
+| `ref_name` | `Runs::create` | `supersede_existing`, web handlers, bootstrap response |
+| `sha` | `Runs::create` | `read_meta`, bootstrap response, web handlers |
+| `pushed_at_ms` | `Runs::create` | `read_meta`, web handlers |
+| `state` | `Runs::create` (→ `pending`) + every transition | everywhere |
+| `failure_kind` | `Run::transition(Failed, …)`, `reconcile_orphans` | web handlers |
+| `queued_at_ms` | `Runs::create` | web handlers |
+| `started_at_ms` | `transition(Active)`, also stamped as fallback in `Complete/Failed/Superseded` | `read_started_at`, web handlers |
+| `finished_at_ms` | `transition(Complete/Failed/Superseded)` | `read_finished_at`, web handlers |
+| `run_token` | `Runs::create` (API sessions only) | `verify_run_token` middleware |
+| `git_dir` | `Run::store_bootstrap_data` (API sessions only) | bootstrap endpoint |
+| `traceparent` | `Run::store_bootstrap_data` (API sessions only) | bootstrap endpoint |
+
+Migration 0007 dropped eight columns that carried no live data with the Process executor: `container_id`, `workspace_path`, `image_tag`, `build_started_at_ms`, `build_finished_at_ms`, `container_started_at_ms`, `container_stopped_at_ms`, and `sentry_trace_id`. The first five were Docker-executor placeholders; `workspace_path` was written at create time but reconstructable from `<base_dir>/<run_id>/workspace`; `sentry_trace_id` was added in migration 0004 and superseded by `traceparent` before it was ever used.
### `jobs` table
-All six columns (`run_id`, `job_id`, `state`, `exit_code`, `started_at_ms`, `finished_at_ms`) are written by `Run::ingest_events` and read by the web detail view. All **live**.
+All six columns (`run_id`, `job_id`, `state`, `exit_code`, `started_at_ms`, `finished_at_ms`) are written by `Run::ingest_events` and read by the web detail view. All live.
The schema permits six states (`pending`, `active`, `complete`, `failed`, `skipped`, `aborted`) but `ingest_events` only writes `complete` and `failed`. The other four states have no producer today — see Gaps below.
### `sh_events` table
-All columns (`run_id`, `job_id`, `started_at_ms`, `finished_at_ms`, `exit_code`, `cmd`) are written by `Run::ingest_events` (pass 2) and read by the web detail view. All **live**.
+All columns (`run_id`, `job_id`, `started_at_ms`, `finished_at_ms`, `exit_code`, `cmd`) are written by `Run::ingest_events` (pass 2) and read by the web detail view. All live.
## Gaps
diff --git a/quire-server/migrations/0007_schema_cleanup.sql b/quire-server/migrations/0007_schema_cleanup.sql
new file mode 100644
index 0000000..c473652
--- /dev/null
+++ b/quire-server/migrations/0007_schema_cleanup.sql
@@ -0,0 +1,49 @@
+-- Drop dead-weight columns from runs: container_id, workspace_path,
+-- image_tag, build_started_at_ms, build_finished_at_ms,
+-- container_started_at_ms, container_stopped_at_ms, sentry_trace_id.
+--
+-- container_id appears in CHECK constraints, so we must recreate the table
+-- rather than using ALTER TABLE DROP COLUMN.
+
+CREATE TABLE runs_new (
+ id TEXT PRIMARY KEY,
+ repo TEXT NOT NULL,
+ ref_name TEXT NOT NULL,
+ sha TEXT NOT NULL,
+ pushed_at_ms INTEGER NOT NULL,
+ state TEXT NOT NULL,
+ failure_kind TEXT,
+ queued_at_ms INTEGER NOT NULL,
+ started_at_ms INTEGER,
+ finished_at_ms INTEGER,
+ run_token TEXT,
+ git_dir TEXT,
+ traceparent TEXT,
+
+ CHECK (state IN ('pending', 'active', 'complete', 'failed', 'superseded')),
+
+ CHECK (started_at_ms IS NULL OR started_at_ms >= queued_at_ms),
+ CHECK (finished_at_ms IS NULL OR finished_at_ms >= queued_at_ms),
+ CHECK (finished_at_ms IS NULL OR started_at_ms IS NULL
+ OR finished_at_ms >= started_at_ms),
+
+ CHECK (CASE state
+ WHEN 'pending' THEN started_at_ms IS NULL AND finished_at_ms IS NULL
+ WHEN 'active' THEN started_at_ms IS NOT NULL AND finished_at_ms IS NULL
+ WHEN 'complete' THEN started_at_ms IS NOT NULL AND finished_at_ms IS NOT NULL
+ WHEN 'failed' THEN finished_at_ms IS NOT NULL
+ WHEN 'superseded' THEN finished_at_ms IS NOT NULL
+ END)
+);
+
+INSERT INTO runs_new
+ SELECT id, repo, ref_name, sha, pushed_at_ms, state, failure_kind,
+ queued_at_ms, started_at_ms, finished_at_ms,
+ run_token, git_dir, traceparent
+ FROM runs;
+
+DROP TABLE runs;
+ALTER TABLE runs_new RENAME TO runs;
+
+CREATE INDEX runs_repo_pushed_at ON runs(repo, pushed_at_ms DESC);
+CREATE INDEX runs_state ON runs(state);
diff --git a/quire-server/src/bin/quire/commands/dev.rs b/quire-server/src/bin/quire/commands/dev.rs
index 14a64be..0add0df 100644
--- a/quire-server/src/bin/quire/commands/dev.rs
+++ b/quire-server/src/bin/quire/commands/dev.rs
@@ -106,7 +106,6 @@ impl Seeder {
fn insert_run(&self, run: &SeedRun) -> Result<()> {
let repo = "example.git";
- let workspace = "/tmp/quire-seed";
// v7 UUIDs so the IDs are time-sortable in addition to unique.
let run_id = Uuid::now_v7().to_string();
@@ -114,25 +113,24 @@ impl Seeder {
let started_at = run.started_delta_ms.map(|d| pushed_at + d);
let finished_at = started_at.zip(run.duration_ms).map(|(s, d)| s + d);
- self.db.execute(
- "INSERT INTO runs (id, repo, ref_name, sha, pushed_at_ms, state, failure_kind,
- queued_at_ms, started_at_ms, finished_at_ms,
- container_id, image_tag, build_started_at_ms, build_finished_at_ms,
- container_started_at_ms, container_stopped_at_ms, workspace_path)
- VALUES (?1, ?2, ?3, ?4, ?5, ?6, NULL, ?7, ?8, ?9, NULL, NULL, NULL, NULL, NULL, NULL, ?10)",
- params![
- run_id,
- repo,
- run.ref_name,
- run.sha,
- pushed_at,
- run.state,
- pushed_at, // queued_at_ms = pushed_at_ms
- started_at,
- finished_at,
- workspace,
- ],
- ).into_diagnostic()?;
+ self.db
+ .execute(
+ "INSERT INTO runs (id, repo, ref_name, sha, pushed_at_ms, state, failure_kind,
+ queued_at_ms, started_at_ms, finished_at_ms)
+ VALUES (?1, ?2, ?3, ?4, ?5, ?6, NULL, ?7, ?8, ?9)",
+ params![
+ run_id,
+ repo,
+ run.ref_name,
+ run.sha,
+ pushed_at,
+ run.state,
+ pushed_at, // queued_at_ms = pushed_at_ms
+ started_at,
+ finished_at,
+ ],
+ )
+ .into_diagnostic()?;
let Some(run_started_at) = started_at else {
return Ok(()); // pending run; no jobs to insert.
diff --git a/quire-server/src/ci/run.rs b/quire-server/src/ci/run.rs
index ad281c6..262cd53 100644
--- a/quire-server/src/ci/run.rs
+++ b/quire-server/src/ci/run.rs
@@ -89,9 +89,7 @@ impl Runs {
/// Create a new run record in the `pending` state.
///
/// Before inserting, supersedes any existing `pending` or `active`
- /// run for the same `(repo, ref)`. Pending runs are marked
- /// superseded directly; active runs have their container killed
- /// first, then marked superseded.
+ /// run for the same `(repo, ref)`.
///
/// Inserts a row into `runs` and creates the run directory for
/// workspace materialization and log storage.
@@ -108,7 +106,6 @@ impl Runs {
(id, Some(s.run_token.as_str()))
}
};
- let workspace_path = self.base_dir.join(&id).join("workspace");
let db = crate::db::open(&self.db_path)?;
@@ -118,8 +115,8 @@ impl Runs {
self.supersede_existing(&db, &meta.r#ref)?;
db.execute(
- "INSERT INTO runs (id, repo, ref_name, sha, pushed_at_ms, state, queued_at_ms, workspace_path, run_token)
- VALUES (?1, ?2, ?3, ?4, ?5, 'pending', ?6, ?7, ?8)",
+ "INSERT INTO runs (id, repo, ref_name, sha, pushed_at_ms, state, queued_at_ms, run_token)
+ VALUES (?1, ?2, ?3, ?4, ?5, 'pending', ?6, ?7)",
rusqlite::params![
&id,
&self.repo,
@@ -127,15 +124,12 @@ impl Runs {
&meta.sha,
meta.pushed_at.as_millisecond(),
Timestamp::now().as_millisecond(),
- workspace_path.to_str().ok_or_else(|| std::io::Error::new(
- std::io::ErrorKind::InvalidData,
- "workspace path is not valid UTF-8",
- ))?,
run_token_str,
],
)?;
// Create run directory for workspace and logs.
+ let workspace_path = self.base_dir.join(&id).join("workspace");
fs_err::create_dir_all(&workspace_path)?;
Ok(Run {
@@ -147,44 +141,26 @@ impl Runs {
}
/// Supersede any existing `pending` or `active` run for
- /// `(repo, ref)`.
- ///
- /// Pending runs are transitioned directly to `superseded`. Active
- /// runs have their container killed via `docker kill` before
- /// transition. Different refs are unaffected.
+ /// `(repo, ref)`. Different refs are unaffected.
fn supersede_existing(&self, db: &rusqlite::Connection, ref_name: &str) -> Result<()> {
let now = Timestamp::now().as_millisecond();
- // Handle active runs first: kill the container, then mark superseded.
- let active_rows: Vec<(String, Option<String>)> = db
+ let active_ids: Vec<String> = db
.prepare(
- "SELECT id, container_id FROM runs
+ "SELECT id FROM runs
WHERE repo = ?1 AND ref_name = ?2 AND state = 'active'",
)?
- .query_map(rusqlite::params![&self.repo, ref_name], |row| {
- Ok((row.get(0)?, row.get(1)?))
- })?
+ .query_map(rusqlite::params![&self.repo, ref_name], |row| row.get(0))?
.collect::<std::result::Result<_, _>>()?;
- for (run_id, container_id) in &active_rows {
- if let Some(cid) = container_id {
- tracing::info!(run_id = %run_id, container_id = %cid, "killing superseded container");
- let kill_status = std::process::Command::new("docker")
- .args(["kill", cid])
- .status();
- if let Err(e) = kill_status {
- tracing::warn!(run_id = %run_id, error = %e, "docker kill failed");
- }
- }
+ for run_id in &active_ids {
db.execute(
- "UPDATE runs SET state = 'superseded', finished_at_ms = ?1, container_id = NULL
- WHERE id = ?2",
+ "UPDATE runs SET state = 'superseded', finished_at_ms = ?1 WHERE id = ?2",
rusqlite::params![now, run_id],
)?;
tracing::info!(run_id = %run_id, "superseded active run");
}
- // Handle pending runs: just mark superseded.
let pending_count = db.execute(
"UPDATE runs SET state = 'superseded', finished_at_ms = ?1
WHERE repo = ?2 AND ref_name = ?3 AND state = 'pending'",
@@ -206,8 +182,7 @@ pub fn reconcile_orphans(db_path: &Path) -> Result<()> {
let now = Timestamp::now().as_millisecond();
let db = crate::db::open(db_path)?;
let count = db.execute(
- "UPDATE runs SET state = 'failed', finished_at_ms = ?1,
- container_id = NULL, failure_kind = 'orphaned'
+ "UPDATE runs SET state = 'failed', finished_at_ms = ?1, failure_kind = 'orphaned'
WHERE state IN ('pending', 'active')",
rusqlite::params![now],
)?;
@@ -541,8 +516,7 @@ impl Run {
db.execute(
"UPDATE runs SET state = ?1, \
started_at_ms = COALESCE(started_at_ms, ?2), \
- finished_at_ms = COALESCE(finished_at_ms, ?3), \
- container_id = NULL \
+ finished_at_ms = COALESCE(finished_at_ms, ?3) \
WHERE id = ?4",
rusqlite::params![to.as_str(), now, now, &self.id],
)?;
@@ -552,7 +526,6 @@ impl Run {
"UPDATE runs SET state = 'failed', \
started_at_ms = COALESCE(started_at_ms, ?1), \
finished_at_ms = COALESCE(finished_at_ms, ?2), \
- container_id = NULL, \
failure_kind = COALESCE(failure_kind, ?3) \
WHERE id = ?4",
rusqlite::params![now, now, failure_kind, &self.id],
diff --git a/quire-server/src/db.rs b/quire-server/src/db.rs
index c43d1f8..e26819d 100644
--- a/quire-server/src/db.rs
+++ b/quire-server/src/db.rs
@@ -18,6 +18,7 @@ static MIGRATIONS: std::sync::LazyLock<Migrations<'static>> = std::sync::LazyLoc
M::up(include_str!("../migrations/0004_bootstrap_api.sql")),
M::up(include_str!("../migrations/0005_rename_run_token.sql")),
M::up(include_str!("../migrations/0006_traceparent.sql")),
+ M::up(include_str!("../migrations/0007_schema_cleanup.sql")),
])
});
diff --git a/quire-server/src/quire/web/handlers.rs b/quire-server/src/quire/web/handlers.rs
index 0e16ffa..1198460 100644
--- a/quire-server/src/quire/web/handlers.rs
+++ b/quire-server/src/quire/web/handlers.rs
@@ -326,12 +326,11 @@ mod tests {
let db = pool.lock().expect("lock");
db.execute(
"INSERT INTO runs (id, repo, ref_name, sha, pushed_at_ms, state, failure_kind,
- queued_at_ms, started_at_ms, finished_at_ms,
- container_id, image_tag, build_started_at_ms, build_finished_at_ms,
- container_started_at_ms, container_stopped_at_ms, workspace_path)
- VALUES (?1, 'example.git', ?2, ?3, ?4, ?5, NULL, ?4, ?6, ?7, NULL, NULL, NULL, NULL, NULL, NULL, '/tmp/ws')",
+ queued_at_ms, started_at_ms, finished_at_ms)
+ VALUES (?1, 'example.git', ?2, ?3, ?4, ?5, NULL, ?4, ?6, ?7)",
rusqlite::params![id, ref_name, sha, queued, state, started, finished],
- ).expect("insert run");
+ )
+ .expect("insert run");
}
fn insert_job(