diff --git a/CHANGELOG.md b/CHANGELOG.md index 225d585d6..527ccb97f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -160,6 +160,18 @@ Releases before v0.27.0 predate the changelog. limit or a suspension — GitHub does not redeliver, so they are recorded and wait at claim; only a `deleted` namespace refuses them. Namespaces with no limit rows behave as before. +- Check-run updates now use a durable, coalescing transactional-outbox + projector and one leased background sender. GitHub API retries, rate-limit + backoff, stale-id reconciliation and restart recovery happen outside + webhook/request paths on both SQLite and PostgreSQL. Each check run carries + an `external_id` of `{run_id}:{job_id}`, and crash-after-POST + reconciliation adopts only a check with that id — never another workflow's + or app's same-named check. The control schema moves to **SQLite v4 / + Postgres v5** in place: an existing control database at the old version is + refused and must be recreated. The Postgres backend must **connect + directly, not through a transaction pooler** (e.g. PgBouncer in transaction + mode): it relies on per-connection `search_path` and on a dedicated + `LISTEN` connection, which transaction pooling breaks. ### Changed diff --git a/crates/preloop-runner-server/src/bootstrap.rs b/crates/preloop-runner-server/src/bootstrap.rs index 614b14fe6..f2ea0dfec 100644 --- a/crates/preloop-runner-server/src/bootstrap.rs +++ b/crates/preloop-runner-server/src/bootstrap.rs @@ -511,13 +511,6 @@ pub async fn reap_once(shared: &Arc) { reason: Some("fork-PR approval window expired".to_owned()), }) .await; - crate::github::report_check_run_completed( - shared, - *run_id, - &job_id, - ExecutionStatus::Failure, - ) - .await; } shared .state @@ -541,13 +534,6 @@ pub async fn reap_once(shared: &Arc) { reason: Some(job.reason.clone()), }) .await; - crate::github::report_check_run_completed( - shared, - job.run_id, - &job.job_id, - ExecutionStatus::Failure, - ) - .await; } // Publish each affected run's updated status. A run the sweep just @@ -679,8 +665,9 @@ async fn run_history_archiver(shared: Arc) { } } -/// Outbox rows are only needed for the live fan-out window (no durable -/// consumer reads them yet): older rows are deleted. Env: +/// Outbox rows are pruned once older than the retention window *and* at or +/// below the slowest durable consumer bookmark (the `check-runs` projector); +/// rows a consumer has not read yet are retained however old they are. Env: /// `PRELOOP_OUTBOX_RETENTION_SECONDS` (default 3600). fn outbox_retention() -> Duration { Duration::from_secs( @@ -1694,6 +1681,10 @@ pub async fn serve(config: ServerConfig) -> anyhow::Result<()> { state: state.clone(), shutdown: shutdown.clone(), }); + let check_run_sender_shared = shared.clone(); + tokio::spawn(async move { + crate::github::run_check_run_sender(check_run_sender_shared).await; + }); // 5s sampler — clone needed state under lock, release, then publish. let sampler_shared = shared.clone(); diff --git a/crates/preloop-runner-server/src/check_run_outbox.rs b/crates/preloop-runner-server/src/check_run_outbox.rs new file mode 100644 index 000000000..a7d166b77 --- /dev/null +++ b/crates/preloop-runner-server/src/check_run_outbox.rs @@ -0,0 +1,68 @@ +//! Durable check-run projection and sender support. +//! +//! The projector is deliberately separate from the GitHub client: it only +//! reads committed outbox rows and writes the coalesced desired-state table. + +use std::sync::{Arc, Weak}; +use std::time::Duration; +use tokio::sync::Notify; + +use crate::control::Backend; + +const BATCH: usize = 256; +const LEASE: Duration = Duration::from_secs(30); +const POLL: Duration = Duration::from_millis(250); + +/// Start the durable projector for either backend. PostgreSQL notifications are +/// converted into the same local dirty signal used by SQLite and by writers on +/// this node; polling remains the recovery path for a lost notification. +pub(crate) fn spawn_consumer(backend: &Arc, dirty: Arc) { + if let Some(mut notifications) = backend.subscribe_event_notifications() { + let wake = Arc::clone(&dirty); + tokio::spawn(async move { + loop { + match notifications.recv().await { + Ok(_) => wake.notify_one(), + Err(tokio::sync::broadcast::error::RecvError::Lagged(_)) => wake.notify_one(), + Err(tokio::sync::broadcast::error::RecvError::Closed) => break, + } + } + }); + } + tokio::spawn(run_consumer(Arc::downgrade(backend), dirty)); +} + +async fn run_consumer(backend: Weak, dirty: Arc) { + let owner = uuid::Uuid::new_v4().to_string(); + loop { + let Some(backend_ref) = backend.upgrade() else { + return; + }; + let mut processed = 0usize; + loop { + match backend_ref + .consume_check_run_outbox(&owner, LEASE, BATCH) + .await + { + Ok(n) => { + processed += n; + if n < BATCH { + break; + } + } + Err(error) => { + tracing::warn!(?error, "check-run outbox projection failed"); + break; + } + } + } + drop(backend_ref); + if processed > 0 { + continue; + } + tokio::select! { + _ = dirty.notified() => {}, + _ = tokio::time::sleep(POLL) => {}, + } + } +} diff --git a/crates/preloop-runner-server/src/control/backend.rs b/crates/preloop-runner-server/src/control/backend.rs index adfbca130..46d4c812d 100644 --- a/crates/preloop-runner-server/src/control/backend.rs +++ b/crates/preloop-runner-server/src/control/backend.rs @@ -491,6 +491,107 @@ pub(crate) trait ControlBackend: Send + Sync { limit: usize, ) -> Result; + /// Consume one durable outbox batch for the check-run projector. The + /// bookmark and all desired-state upserts commit atomically while the + /// named consumer lease is held by `owner`. + async fn consume_check_run_outbox( + &self, + owner: &str, + lease_for: std::time::Duration, + limit: usize, + ) -> Result; + + /// Lease due desired check-run rows for the single background sender. + async fn lease_check_run_updates( + &self, + owner: &str, + lease_for: std::time::Duration, + limit: usize, + ) -> Result, ControlError>; + + /// Save a GitHub check-run id obtained by the sender. A newer desired + /// version may have arrived meanwhile; the conditional update preserves + /// that newer row while still recording the id. + async fn set_check_run_update_id( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + version: i64, + check_run_id: u64, + ) -> Result<(), ControlError>; + + /// Complete a leased row only when no newer desired version arrived. + async fn finish_check_run_update( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + version: i64, + ) -> Result<(), ControlError>; + + /// Release a failed sender lease with exponential-backoff metadata. + async fn retry_check_run_update( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + delay: std::time::Duration, + permanent: bool, + ) -> Result<(), ControlError>; + + /// Clear a stale GitHub id so the next attempt recreates/reconciles it. + async fn clear_check_run_update_id( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + expected: u64, + ) -> Result<(), ControlError>; + + async fn enqueue_check_run_update( + &self, + update: CheckRunUpdateInput, + ) -> Result<(), ControlError>; + + /// Release a rate-limited lease without counting a delivery attempt. + async fn defer_check_run_update( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + delay: std::time::Duration, + ) -> Result<(), ControlError>; + + /// Extend this sender's lease on one row to `now + lease_for`. `false` + /// when the row is no longer leased by `owner` (another sender took it + /// over, or it was settled): the caller must not call GitHub for it. + async fn renew_check_run_update( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + lease_for: std::time::Duration, + ) -> Result; + + /// Append a durable projection wake after a reporter stamped + /// `reports_check_runs`. + async fn append_check_run_projection( + &self, + run_id: RunId, + job_id: Option<&JobId>, + ) -> Result<(), ControlError>; + + /// Current end of the outbox stream (backend safe point). + async fn outbox_head(&self) -> Result, ControlError>; + + /// Up to `limit` outbox rows after `after`, in backend commit order. + async fn outbox_read( + &self, + after: OutboxBookmark, + limit: usize, + ) -> Result, ControlError>; + /// One reaper tick's inputs, read directly (no working-set load, no lock). async fn reap_inputs(&self) -> Result; @@ -1297,24 +1398,23 @@ impl Backend { } } - /// The outbox position a consumer that starts now reads from. `None` - /// on SQLite. + /// The current end of the outbox stream. SQLite returns event-id order; + /// PostgreSQL returns its `(txid,event_id)` safe point. pub(crate) async fn outbox_head(&self) -> Result, ControlError> { match self { - Self::Sqlite(_) => Ok(None), + Self::Sqlite(b) => b.outbox_head().await.map(Some), Self::Postgres(b) => b.outbox_head().await.map(Some), } } - /// Up to `limit` outbox rows after `after`, from finished transactions, - /// in `(txid, event_id)` order. Empty on SQLite. + /// Up to `limit` rows after `after`, ordered by the backend safe point. pub(crate) async fn outbox_read( &self, after: OutboxBookmark, limit: usize, ) -> Result, ControlError> { match self { - Self::Sqlite(_) => Ok(Vec::new()), + Self::Sqlite(b) => b.outbox_read(after, limit).await, Self::Postgres(b) => b.outbox_read(after, limit).await, } } @@ -1327,6 +1427,163 @@ impl Backend { } } + pub(crate) async fn consume_check_run_outbox( + &self, + owner: &str, + lease_for: std::time::Duration, + limit: usize, + ) -> Result { + match self { + Self::Sqlite(b) => b.consume_check_run_outbox(owner, lease_for, limit).await, + Self::Postgres(b) => b.consume_check_run_outbox(owner, lease_for, limit).await, + } + } + + pub(crate) async fn lease_check_run_updates( + &self, + owner: &str, + lease_for: std::time::Duration, + limit: usize, + ) -> Result, ControlError> { + match self { + Self::Sqlite(b) => b.lease_check_run_updates(owner, lease_for, limit).await, + Self::Postgres(b) => b.lease_check_run_updates(owner, lease_for, limit).await, + } + } + + pub(crate) async fn set_check_run_update_id( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + version: i64, + check_run_id: u64, + ) -> Result<(), ControlError> { + match self { + Self::Sqlite(b) => { + b.set_check_run_update_id(owner, run_id, job_id, version, check_run_id) + .await + } + Self::Postgres(b) => { + b.set_check_run_update_id(owner, run_id, job_id, version, check_run_id) + .await + } + } + } + + pub(crate) async fn finish_check_run_update( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + version: i64, + ) -> Result<(), ControlError> { + match self { + Self::Sqlite(b) => { + b.finish_check_run_update(owner, run_id, job_id, version) + .await + } + Self::Postgres(b) => { + b.finish_check_run_update(owner, run_id, job_id, version) + .await + } + } + } + + pub(crate) async fn retry_check_run_update( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + delay: std::time::Duration, + permanent: bool, + ) -> Result<(), ControlError> { + match self { + Self::Sqlite(b) => { + b.retry_check_run_update(owner, run_id, job_id, delay, permanent) + .await + } + Self::Postgres(b) => { + b.retry_check_run_update(owner, run_id, job_id, delay, permanent) + .await + } + } + } + + pub(crate) async fn clear_check_run_update_id( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + expected: u64, + ) -> Result<(), ControlError> { + match self { + Self::Sqlite(b) => { + b.clear_check_run_update_id(owner, run_id, job_id, expected) + .await + } + Self::Postgres(b) => { + b.clear_check_run_update_id(owner, run_id, job_id, expected) + .await + } + } + } + + pub(crate) async fn defer_check_run_update( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + delay: std::time::Duration, + ) -> Result<(), ControlError> { + match self { + Self::Sqlite(b) => b.defer_check_run_update(owner, run_id, job_id, delay).await, + Self::Postgres(b) => b.defer_check_run_update(owner, run_id, job_id, delay).await, + } + } + + pub(crate) async fn renew_check_run_update( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + lease_for: std::time::Duration, + ) -> Result { + match self { + Self::Sqlite(b) => { + b.renew_check_run_update(owner, run_id, job_id, lease_for) + .await + } + Self::Postgres(b) => { + b.renew_check_run_update(owner, run_id, job_id, lease_for) + .await + } + } + } + + /// Append a durable projection wake after a reporter stamped + /// `reports_check_runs`; see `append_check_run_projection` on the + /// backends. + pub(crate) async fn append_check_run_projection( + &self, + run_id: RunId, + job_id: Option<&JobId>, + ) -> Result<(), ControlError> { + match self { + Self::Sqlite(b) => b.append_check_run_projection(run_id, job_id).await, + Self::Postgres(b) => b.append_check_run_projection(run_id, job_id).await, + } + } + pub(crate) async fn enqueue_check_run_update( + &self, + update: CheckRunUpdateInput, + ) -> Result<(), ControlError> { + match self { + Self::Sqlite(b) => b.enqueue_check_run_update(update).await, + Self::Postgres(b) => b.enqueue_check_run_update(update).await, + } + } + /// `#[cfg(test)]` working-set snapshot for test assertions. Each /// backend rebuilds the old field names from its own tables. #[cfg(any(test, feature = "test-support"))] @@ -1812,6 +2069,167 @@ impl ControlBackend for Backend { Self::Postgres(b) => b.prune_outbox(older_than, limit).await, } } + async fn consume_check_run_outbox( + &self, + owner: &str, + lease_for: std::time::Duration, + limit: usize, + ) -> Result { + match self { + Self::Sqlite(b) => b.consume_check_run_outbox(owner, lease_for, limit).await, + Self::Postgres(b) => b.consume_check_run_outbox(owner, lease_for, limit).await, + } + } + async fn lease_check_run_updates( + &self, + owner: &str, + lease_for: std::time::Duration, + limit: usize, + ) -> Result, ControlError> { + match self { + Self::Sqlite(b) => b.lease_check_run_updates(owner, lease_for, limit).await, + Self::Postgres(b) => b.lease_check_run_updates(owner, lease_for, limit).await, + } + } + async fn set_check_run_update_id( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + version: i64, + check_run_id: u64, + ) -> Result<(), ControlError> { + match self { + Self::Sqlite(b) => { + b.set_check_run_update_id(owner, run_id, job_id, version, check_run_id) + .await + } + Self::Postgres(b) => { + b.set_check_run_update_id(owner, run_id, job_id, version, check_run_id) + .await + } + } + } + async fn finish_check_run_update( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + version: i64, + ) -> Result<(), ControlError> { + match self { + Self::Sqlite(b) => { + b.finish_check_run_update(owner, run_id, job_id, version) + .await + } + Self::Postgres(b) => { + b.finish_check_run_update(owner, run_id, job_id, version) + .await + } + } + } + async fn retry_check_run_update( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + delay: std::time::Duration, + permanent: bool, + ) -> Result<(), ControlError> { + match self { + Self::Sqlite(b) => { + b.retry_check_run_update(owner, run_id, job_id, delay, permanent) + .await + } + Self::Postgres(b) => { + b.retry_check_run_update(owner, run_id, job_id, delay, permanent) + .await + } + } + } + async fn clear_check_run_update_id( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + expected: u64, + ) -> Result<(), ControlError> { + match self { + Self::Sqlite(b) => { + b.clear_check_run_update_id(owner, run_id, job_id, expected) + .await + } + Self::Postgres(b) => { + b.clear_check_run_update_id(owner, run_id, job_id, expected) + .await + } + } + } + async fn enqueue_check_run_update( + &self, + update: CheckRunUpdateInput, + ) -> Result<(), ControlError> { + match self { + Self::Sqlite(b) => b.enqueue_check_run_update(update).await, + Self::Postgres(b) => b.enqueue_check_run_update(update).await, + } + } + async fn defer_check_run_update( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + delay: std::time::Duration, + ) -> Result<(), ControlError> { + match self { + Self::Sqlite(b) => b.defer_check_run_update(owner, run_id, job_id, delay).await, + Self::Postgres(b) => b.defer_check_run_update(owner, run_id, job_id, delay).await, + } + } + async fn renew_check_run_update( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + lease_for: std::time::Duration, + ) -> Result { + match self { + Self::Sqlite(b) => { + b.renew_check_run_update(owner, run_id, job_id, lease_for) + .await + } + Self::Postgres(b) => { + b.renew_check_run_update(owner, run_id, job_id, lease_for) + .await + } + } + } + async fn append_check_run_projection( + &self, + run_id: RunId, + job_id: Option<&JobId>, + ) -> Result<(), ControlError> { + match self { + Self::Sqlite(b) => b.append_check_run_projection(run_id, job_id).await, + Self::Postgres(b) => b.append_check_run_projection(run_id, job_id).await, + } + } + async fn outbox_head(&self) -> Result, ControlError> { + match self { + Self::Sqlite(b) => b.outbox_head().await.map(Some), + Self::Postgres(b) => b.outbox_head().await.map(Some), + } + } + async fn outbox_read( + &self, + after: OutboxBookmark, + limit: usize, + ) -> Result, ControlError> { + match self { + Self::Sqlite(b) => b.outbox_read(after, limit).await, + Self::Postgres(b) => b.outbox_read(after, limit).await, + } + } async fn runner_exists(&self, runner_id: i64) -> Result { match self { Self::Sqlite(b) => b.runner_exists(runner_id).await, diff --git a/crates/preloop-runner-server/src/control/lite/check_runs.rs b/crates/preloop-runner-server/src/control/lite/check_runs.rs new file mode 100644 index 000000000..bada78bef --- /dev/null +++ b/crates/preloop-runner-server/src/control/lite/check_runs.rs @@ -0,0 +1,202 @@ +use super::{LiteBackend, db}; +use crate::control::types::{CheckRunUpdate, ControlError, OutboxBookmark, OutboxRow}; +use preloop_gha_protocol::{JobId, RunId}; +use rusqlite::{Transaction, params}; +use serde_json::Value; + +const CONSUMER: &str = "check-runs"; +fn relevant(topic: &str) -> bool { + matches!( + topic, + "run.created.v1" + | "run.completed.v1" + | "expansion.queued.v1" + | "job.queued.v1" + | "job.started.v1" + | "job.completed.v1" + | "job_status.v1" + | "run_status.v1" + | "check_run_projection.v1" + ) +} +fn run_text(run: RunId) -> String { + run.0.to_string() +} +fn now_us() -> i64 { + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_micros() as i64) + .unwrap_or(0) +} + +impl LiteBackend { + pub(crate) async fn enqueue_check_run_update( + &self, + update: crate::control::types::CheckRunUpdateInput, + ) -> Result<(), ControlError> { + self.write(|tx|{tx.execute("INSERT INTO check_run_updates(run_id,job_id,installation_id,check_run_id,version,payload,not_before,attempts) VALUES(?1,?2,?3,?4,?5,?6,?7,0) ON CONFLICT(run_id,job_id) DO UPDATE SET payload=excluded.payload,version=excluded.version,not_before=excluded.not_before WHERE excluded.version>check_run_updates.version",params![run_text(update.run_id),update.job_id.0,update.installation_id,update.check_run_id.map(|v|v as i64),update.version,update.payload.to_string(),now_us()]).map_err(db)?;Ok(())}) + } + pub(crate) async fn consume_check_run_outbox( + &self, + owner: &str, + lease_for: std::time::Duration, + limit: usize, + ) -> Result { + self.write(|tx|{ + let now=now_us(); + tx.execute("INSERT INTO consumer_offsets(consumer_name,last_event_id) VALUES(?1,0) ON CONFLICT(consumer_name) DO NOTHING",params![CONSUMER]).map_err(db)?; + let lease=now+lease_for.as_micros() as i64; + let n=tx.execute("UPDATE consumer_offsets SET lease_owner=?2,lease_until=?3,updated_at=?4 WHERE consumer_name=?1 AND (lease_owner IS NULL OR lease_until?1 ORDER BY event_id LIMIT ?2").map_err(db)?; + let it=st.query_map(params![after,limit as i64],|r|{ + let run_id=match r.get::<_,Option>(1)? { + Some(v)=>Some(v.parse::().map(RunId).map_err(|_|rusqlite::Error::InvalidQuery)?), + None=>None, + }; + Ok(OutboxRow{bookmark:OutboxBookmark{txid:0,event_id:r.get(0)?},run_id,job_id:r.get(2)?,version:r.get(3)?,origin:r.get(4)?,topic:r.get(5)?,payload:serde_json::from_str::(&r.get::<_,String>(6)?).map_err(|_|rusqlite::Error::InvalidQuery)?}) + }).map_err(db)?; + for row in it { rows.push(row.map_err(db)?); } + } + let count=rows.len(); let mut last=after; + for row in rows { last=row.bookmark.event_id; if relevant(&row.topic){project_row(tx,&row)?;} } + if count>0 { tx.execute("UPDATE consumer_offsets SET last_event_id=?2,updated_at=?3 WHERE consumer_name=?1",params![CONSUMER,last,now]).map_err(db)?; } + Ok(count) + }) + } + pub(crate) async fn lease_check_run_updates( + &self, + owner: &str, + lease_for: std::time::Duration, + limit: usize, + ) -> Result, ControlError> { + self.write(|tx|{let now=now_us();let until=now+lease_for.as_micros() as i64;let mut st=tx.prepare("SELECT run_id,job_id,installation_id,check_run_id,version,payload,attempts FROM check_run_updates WHERE not_before<=?1 AND (leased_until IS NULL OR leased_until(0)?,r.get::<_,String>(1)?,r.get::<_,i64>(2)?,r.get::<_,Option>(3)?,r.get::<_,i64>(4)?,r.get::<_,String>(5)?,r.get::<_,i32>(6)?))).map_err(db)?;let mut out=Vec::new();for row in rows{let (run,job,inst,id,ver,payload,attempts)=row.map_err(db)?;tx.execute("UPDATE check_run_updates SET lease_owner=?3,leased_until=?4 WHERE run_id=?1 AND job_id=?2",params![run,job,owner,until]).map_err(db)?;out.push(CheckRunUpdate{run_id:run.parse().map(RunId).map_err(|_|ControlError::backend(anyhow::anyhow!("bad run id")))?,job_id:JobId(job),installation_id:inst,check_run_id:id.map(|v|v as u64),version:ver,payload:serde_json::from_str(&payload).map_err(ControlError::backend)?,attempts});}Ok(out)}) + } + pub(crate) async fn set_check_run_update_id( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + version: i64, + id: u64, + ) -> Result<(), ControlError> { + self.write(|tx|{tx.execute("UPDATE check_run_updates SET check_run_id=?,leased_until=? WHERE lease_owner=? AND run_id=? AND job_id=? AND version=?",params![id as i64,now_us()+300_000_000,owner,run_text(run_id),job_id.0,version]).map_err(db)?;Ok(())}) + } + pub(crate) async fn finish_check_run_update( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + version: i64, + ) -> Result<(), ControlError> { + self.write(|tx|{tx.execute("DELETE FROM check_run_updates WHERE lease_owner=? AND run_id=? AND job_id=? AND version=?",params![owner,run_text(run_id),job_id.0,version]).map_err(db)?;Ok(())}) + } + pub(crate) async fn retry_check_run_update( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + delay: std::time::Duration, + permanent: bool, + ) -> Result<(), ControlError> { + self.write(|tx|{if permanent{tx.execute("DELETE FROM check_run_updates WHERE lease_owner=? AND run_id=? AND job_id=?",params![owner,run_text(run_id),job_id.0]).map_err(db)?;}else{tx.execute("UPDATE check_run_updates SET attempts=attempts+1,not_before=?,leased_until=NULL,lease_owner=NULL WHERE lease_owner=? AND run_id=? AND job_id=?",params![now_us()+delay.as_micros() as i64,owner,run_text(run_id),job_id.0]).map_err(db)?;}Ok(())}) + } + pub(crate) async fn clear_check_run_update_id( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + expected: u64, + ) -> Result<(), ControlError> { + self.write(|tx|{tx.execute("UPDATE check_run_updates SET check_run_id=NULL WHERE lease_owner=? AND run_id=? AND job_id=? AND check_run_id=?",params![owner,run_text(run_id),job_id.0,expected as i64]).map_err(db)?;Ok(())}) + } + + /// Release a lease without counting an attempt: the sender is waiting out + /// a rate limit, not failing to deliver. + pub(crate) async fn defer_check_run_update( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + delay: std::time::Duration, + ) -> Result<(), ControlError> { + self.write(|tx|{tx.execute("UPDATE check_run_updates SET not_before=?,leased_until=NULL,lease_owner=NULL WHERE lease_owner=? AND run_id=? AND job_id=?",params![now_us()+delay.as_micros() as i64,owner,run_text(run_id),job_id.0]).map_err(db)?;Ok(())}) + } + + /// Extend `owner`'s lease on one row. A row another sender re-leased has + /// a different `lease_owner`, so ownership alone decides: `false` means + /// the caller lost the row and must not call GitHub for it. + pub(crate) async fn renew_check_run_update( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + lease_for: std::time::Duration, + ) -> Result { + self.write(|tx| { + let renewed = tx + .execute( + "UPDATE check_run_updates SET leased_until=? \ + WHERE lease_owner=? AND run_id=? AND job_id=?", + params![ + now_us() + lease_for.as_micros() as i64, + owner, + run_text(run_id), + job_id.0 + ], + ) + .map_err(db)?; + Ok(renewed == 1) + }) + } + + /// Append a durable projection wake for one run (or one job) *after* a + /// reporter stamped `reports_check_runs`. + /// + /// Unlike a `JobStatus` event this is never stamped `Stale`, so a job + /// already terminal when the run's reporting flag lands (an `if: false` + /// leg concluded during submit) still reaches the projector. + pub(crate) async fn append_check_run_projection( + &self, + run_id: RunId, + job_id: Option<&JobId>, + ) -> Result<(), ControlError> { + self.write(|tx| { + let payload = serde_json::json!({"job_id": job_id.map(|job| job.0.clone())}); + super::jobs::insert_outbox( + tx, + super::jobs::namespace_of(tx, run_id)?.as_str(), + Some(run_id), + job_id.map(|job| job.0.as_str()), + None, + "check_run_projection.v1", + &payload.to_string(), + ) + }) + } +} +fn project_row(tx: &Transaction<'_>, row: &OutboxRow) -> Result<(), ControlError> { + let Some(run_id) = row.run_id else { + return Ok(()); + }; + let mut st=tx.prepare("SELECT r.reports_check_runs,r.repository,COALESCE(ps.effective_sha,json_extract(s.submission,'$.status_check_sha'),json_extract(s.submission,'$.sha'),r.head_sha),j.job_id,j.status,j.version,COALESCE(js.display_name,j.job_id),j.check_run_id,j.kind FROM runs r JOIN jobs j ON j.run_id=r.run_id LEFT JOIN job_specs js ON js.run_id=j.run_id AND js.job_id=j.job_id LEFT JOIN run_submissions s ON s.run_id=r.run_id LEFT JOIN run_push_states ps ON ps.run_id=r.run_id WHERE r.run_id=?1 AND (?2 IS NULL OR j.job_id=?2)").map_err(db)?; + let mut rows = st + .query(params![run_text(run_id), row.job_id.as_deref()]) + .map_err(db)?; + while let Some(r) = rows.next().map_err(db)? { + let reports: bool = r.get(0).map_err(db)?; + let kind: String = r.get(8).map_err(db)?; + if !reports || kind == "matrix_parent" || kind == "reusable_caller" { + continue; + } + let job: String = r.get(3).map_err(db)?; + let ver: i64 = r.get(5).map_err(db)?; + let payload = serde_json::json!({"repository":r.get::<_,String>(1).map_err(db)?,"sha":r.get::<_,String>(2).map_err(db)?,"job_id":job,"status":r.get::<_,String>(4).map_err(db)?,"name":r.get::<_,String>(6).map_err(db)?}); + let id: Option = r.get(7).map_err(db)?; + tx.execute("INSERT INTO check_run_updates(run_id,job_id,installation_id,check_run_id,version,payload,not_before,attempts,leased_until,lease_owner) VALUES(?1,?2,0,?3,?4,?5,?6,0,NULL,NULL) ON CONFLICT(run_id,job_id) DO UPDATE SET payload=excluded.payload,version=excluded.version,check_run_id=COALESCE(excluded.check_run_id,check_run_updates.check_run_id),not_before=excluded.not_before,leased_until=NULL,lease_owner=NULL,attempts=0 WHERE excluded.version>check_run_updates.version",params![run_text(run_id),job,id,ver,payload.to_string(),now_us()]).map_err(db)?; + } + Ok(()) +} diff --git a/crates/preloop-runner-server/src/control/lite/impls.rs b/crates/preloop-runner-server/src/control/lite/impls.rs index b857bbfb7..d1795b1c7 100644 --- a/crates/preloop-runner-server/src/control/lite/impls.rs +++ b/crates/preloop-runner-server/src/control/lite/impls.rs @@ -233,6 +233,107 @@ impl ControlBackend for LiteBackend { ) -> Result { self.prune_outbox(older_than, limit).await } + async fn consume_check_run_outbox( + &self, + owner: &str, + lease_for: std::time::Duration, + limit: usize, + ) -> Result { + self.consume_check_run_outbox(owner, lease_for, limit).await + } + async fn lease_check_run_updates( + &self, + owner: &str, + lease_for: std::time::Duration, + limit: usize, + ) -> Result, ControlError> { + self.lease_check_run_updates(owner, lease_for, limit).await + } + async fn set_check_run_update_id( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + version: i64, + check_run_id: u64, + ) -> Result<(), ControlError> { + self.set_check_run_update_id(owner, run_id, job_id, version, check_run_id) + .await + } + async fn finish_check_run_update( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + version: i64, + ) -> Result<(), ControlError> { + self.finish_check_run_update(owner, run_id, job_id, version) + .await + } + async fn retry_check_run_update( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + delay: std::time::Duration, + permanent: bool, + ) -> Result<(), ControlError> { + self.retry_check_run_update(owner, run_id, job_id, delay, permanent) + .await + } + async fn clear_check_run_update_id( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + expected: u64, + ) -> Result<(), ControlError> { + self.clear_check_run_update_id(owner, run_id, job_id, expected) + .await + } + async fn enqueue_check_run_update( + &self, + update: CheckRunUpdateInput, + ) -> Result<(), ControlError> { + self.enqueue_check_run_update(update).await + } + async fn defer_check_run_update( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + delay: std::time::Duration, + ) -> Result<(), ControlError> { + self.defer_check_run_update(owner, run_id, job_id, delay) + .await + } + async fn renew_check_run_update( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + lease_for: std::time::Duration, + ) -> Result { + self.renew_check_run_update(owner, run_id, job_id, lease_for) + .await + } + async fn append_check_run_projection( + &self, + run_id: RunId, + job_id: Option<&JobId>, + ) -> Result<(), ControlError> { + self.append_check_run_projection(run_id, job_id).await + } + async fn outbox_head(&self) -> Result, ControlError> { + self.outbox_head().await.map(Some) + } + async fn outbox_read( + &self, + after: OutboxBookmark, + limit: usize, + ) -> Result, ControlError> { + self.outbox_read(after, limit).await + } async fn reap_inputs(&self) -> Result { self.reap_inputs().await } diff --git a/crates/preloop-runner-server/src/control/lite/lifecycle.rs b/crates/preloop-runner-server/src/control/lite/lifecycle.rs index 5c7e91621..44faad691 100644 --- a/crates/preloop-runner-server/src/control/lite/lifecycle.rs +++ b/crates/preloop-runner-server/src/control/lite/lifecycle.rs @@ -1148,8 +1148,11 @@ impl LiteBackend { self.write(|tx| { tx.execute( "DELETE FROM outbox_events WHERE event_id IN ( \ - SELECT event_id FROM outbox_events WHERE created_at < ?1 \ - ORDER BY created_at LIMIT ?2)", + SELECT e.event_id FROM outbox_events e \ + WHERE e.created_at < ?1 AND \ + e.event_id <= COALESCE((SELECT MIN(last_event_id) FROM consumer_offsets), \ + e.event_id) \ + ORDER BY e.created_at LIMIT ?2)", params![cutoff, limit as i64], ) .map(|n| n as u64) @@ -1157,6 +1160,64 @@ impl LiteBackend { }) } + /// The SQLite outbox position at the current end of the single-writer + /// stream. SQLite event ids are commit order, so no txid safe point is + /// needed. + pub(crate) async fn outbox_head(&self) -> Result { + self.read(|tx| { + let event_id: i64 = tx + .query_row( + "SELECT COALESCE(MAX(event_id),0) FROM outbox_events", + [], + |r| r.get(0), + ) + .map_err(db)?; + Ok(OutboxBookmark { txid: 0, event_id }) + }) + } + + pub(crate) async fn outbox_read( + &self, + after: OutboxBookmark, + limit: usize, + ) -> Result, ControlError> { + self.read(|tx| { + let mut stmt = tx + .prepare( + "SELECT event_id,run_id,job_id,version,'' AS origin,topic,payload \ + FROM outbox_events WHERE event_id > ?1 ORDER BY event_id LIMIT ?2", + ) + .map_err(db)?; + let rows = stmt + .query_map(params![after.event_id, limit as i64], |row| { + let run_id = match row.get::<_, Option>(1)? { + Some(value) => Some( + value + .parse() + .map(RunId) + .map_err(|_| rusqlite::Error::InvalidQuery)?, + ), + None => None, + }; + Ok(OutboxRow { + bookmark: OutboxBookmark { + txid: 0, + event_id: row.get(0)?, + }, + run_id, + job_id: row.get(2)?, + version: row.get(3)?, + origin: row.get(4)?, + topic: row.get(5)?, + payload: serde_json::from_str(&row.get::<_, String>(6)?) + .map_err(|_| rusqlite::Error::InvalidQuery)?, + }) + }) + .map_err(db)?; + rows.collect::, _>>().map_err(db) + }) + } + /// `reconcile_on_boot` (pg lifecycle.rs): drop holds/waits of dead or /// terminal runs, requeue claimed jobs whose session is gone, reset /// `expanding` nodes to `pending_expansion`. diff --git a/crates/preloop-runner-server/src/control/lite/mod.rs b/crates/preloop-runner-server/src/control/lite/mod.rs index e192eec6a..e87e82e4a 100644 --- a/crates/preloop-runner-server/src/control/lite/mod.rs +++ b/crates/preloop-runner-server/src/control/lite/mod.rs @@ -13,6 +13,7 @@ //! the SQL mirrors the Postgres backend statement for statement. mod acquire; +mod check_runs; mod codec; mod concurrency; mod dispatch; @@ -48,7 +49,7 @@ use std::time::Duration; /// The schema this build reads and writes. Greenfield: any other stamped /// version is refused at open (no migrations). -pub(crate) const SCHEMA_VERSION: &str = "3"; +pub(crate) const SCHEMA_VERSION: &str = "4"; /// The translated schema (see the file header for the type mapping). const SCHEMA_SQL: &str = include_str!("schema.sql"); diff --git a/crates/preloop-runner-server/src/control/lite/schema.sql b/crates/preloop-runner-server/src/control/lite/schema.sql index d70f35d37..c47e5b7c5 100644 --- a/crates/preloop-runner-server/src/control/lite/schema.sql +++ b/crates/preloop-runner-server/src/control/lite/schema.sql @@ -554,12 +554,13 @@ CREATE TABLE check_run_updates ( job_id TEXT NOT NULL, installation_id INTEGER NOT NULL, check_run_id INTEGER, + version INTEGER NOT NULL DEFAULT 0, payload TEXT NOT NULL, not_before INTEGER NOT NULL DEFAULT (CAST(unixepoch('subsec') * 1000000 AS INTEGER)), attempts INTEGER NOT NULL DEFAULT 0, leased_until INTEGER, - PRIMARY KEY (run_id, job_id), - FOREIGN KEY (run_id, job_id) REFERENCES jobs(run_id, job_id) ON DELETE CASCADE + lease_owner TEXT, + PRIMARY KEY (run_id, job_id) ); CREATE INDEX check_run_updates_queue ON check_run_updates(installation_id, not_before); diff --git a/crates/preloop-runner-server/src/control/pg/check_runs.rs b/crates/preloop-runner-server/src/control/pg/check_runs.rs new file mode 100644 index 000000000..af09e7953 --- /dev/null +++ b/crates/preloop-runner-server/src/control/pg/check_runs.rs @@ -0,0 +1,278 @@ +use super::{PgBackend, codec, db}; +use crate::control::types::{CheckRunUpdate, ControlError, OutboxBookmark, OutboxRow}; +use preloop_gha_protocol::{JobId, RunId}; +use tokio_postgres::Transaction; + +const CONSUMER: &str = "check-runs"; + +fn relevant(topic: &str) -> bool { + matches!( + topic, + "run.created.v1" + | "run.completed.v1" + | "expansion.queued.v1" + | "job.queued.v1" + | "job.started.v1" + | "job.completed.v1" + | "job_status.v1" + | "run_status.v1" + | "check_run_projection.v1" + ) +} + +fn run_text(run: RunId) -> String { + run.0.to_string() +} + +impl PgBackend { + pub(crate) async fn enqueue_check_run_update( + &self, + update: crate::control::types::CheckRunUpdateInput, + ) -> Result<(), ControlError> { + let client = self.writer().await?; + let payload = serde_json::to_string(&update.payload).map_err(ControlError::backend)?; + client.execute("INSERT INTO check_run_updates(run_id,job_id,installation_id,check_run_id,version,payload,not_before,attempts) VALUES($1::text::uuid,$2,$3,$4,$5,$6::text::jsonb,now(),0) ON CONFLICT(run_id,job_id) DO UPDATE SET payload=EXCLUDED.payload,version=EXCLUDED.version,not_before=now() WHERE EXCLUDED.version>check_run_updates.version", &[&update.run_id.0.to_string(),&update.job_id.0,&update.installation_id,&update.check_run_id.map(|v|v as i64),&update.version,&payload]).await.map(|_|()).map_err(db) + } + pub(crate) async fn consume_check_run_outbox( + &self, + owner: &str, + lease_for: std::time::Duration, + limit: usize, + ) -> Result { + let mut client = self.writer().await?; + let tx = client.transaction().await.map_err(db)?; + tx.execute( + "INSERT INTO consumer_offsets (consumer_name,last_txid,last_event_id) VALUES ($1,'0',0) ON CONFLICT DO NOTHING", + &[&CONSUMER], + ).await.map_err(db)?; + let leased = tx.query_opt( + "UPDATE consumer_offsets SET lease_owner=$2, lease_until=now()+make_interval(secs=>$3), updated_at=now() \ + WHERE consumer_name=$1 AND (lease_owner IS NULL OR lease_until < now() OR lease_owner=$2) RETURNING last_txid::text,last_event_id", + &[&CONSUMER, &owner, &lease_for.as_secs_f64()], + ).await.map_err(db)?; + let Some(offset) = leased else { + tx.rollback().await.map_err(db)?; + return Ok(0); + }; + let after = OutboxBookmark { + txid: offset + .get::<_, String>(0) + .parse() + .map_err(ControlError::backend)?, + event_id: offset.get(1), + }; + let rows = read_rows(&tx, after, limit).await?; + let count = rows.len(); + let mut bookmark = after; + for row in rows { + bookmark = row.bookmark; + if relevant(&row.topic) { + project_row(&tx, &row).await?; + } + } + if count != 0 { + tx.execute("UPDATE consumer_offsets SET last_txid=$2::text::xid8,last_event_id=$3,updated_at=now() WHERE consumer_name=$1", &[&CONSUMER, &bookmark.txid.to_string(), &bookmark.event_id]).await.map_err(db)?; + } + tx.commit().await.map_err(db)?; + Ok(count) + } + + pub(crate) async fn lease_check_run_updates( + &self, + owner: &str, + lease_for: std::time::Duration, + limit: usize, + ) -> Result, ControlError> { + let mut client = self.writer().await?; + let tx = client.transaction().await.map_err(db)?; + let rows = tx.query( + "WITH picked AS (SELECT run_id,job_id FROM check_run_updates WHERE not_before<=now() AND (leased_until IS NULL OR leased_until$3) FROM picked p WHERE u.run_id=p.run_id AND u.job_id=p.job_id \ + RETURNING u.run_id::text,u.job_id,u.installation_id,u.check_run_id,u.version,u.payload::text,u.attempts", + &[&owner, &(limit as i64), &lease_for.as_secs_f64()]).await.map_err(db)?; + let out = rows + .into_iter() + .map(|r| { + Ok(CheckRunUpdate { + run_id: codec::run_id(r.get(0))?, + job_id: JobId(r.get(1)), + installation_id: r.get(2), + check_run_id: r.get::<_, Option>(3).map(|v| v as u64), + version: r.get(4), + payload: codec::from_json(r.get::<_, String>(5).as_str())?, + attempts: r.get(6), + }) + }) + .collect::, ControlError>>(); + tx.commit().await.map_err(db)?; + out + } + + pub(crate) async fn set_check_run_update_id( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + version: i64, + id: u64, + ) -> Result<(), ControlError> { + let client = self.writer().await?; + client.execute("UPDATE check_run_updates SET check_run_id=$5,leased_until=now()+interval '5 minutes' WHERE lease_owner=$1 AND run_id=$2::text::uuid AND job_id=$3 AND version=$4", &[&owner,&run_text(run_id),&job_id.0,&version,&(id as i64)]).await.map(|_|()).map_err(db) + } + pub(crate) async fn finish_check_run_update( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + version: i64, + ) -> Result<(), ControlError> { + let client = self.writer().await?; + client.execute("DELETE FROM check_run_updates WHERE lease_owner=$1 AND run_id=$2::text::uuid AND job_id=$3 AND version=$4", &[&owner,&run_text(run_id),&job_id.0,&version]).await.map(|_|()).map_err(db) + } + pub(crate) async fn retry_check_run_update( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + delay: std::time::Duration, + permanent: bool, + ) -> Result<(), ControlError> { + let client = self.writer().await?; + if permanent { + client.execute("DELETE FROM check_run_updates WHERE lease_owner=$1 AND run_id=$2::text::uuid AND job_id=$3", &[&owner,&run_text(run_id),&job_id.0]).await.map_err(db)?; + } else { + client.execute("UPDATE check_run_updates SET attempts=attempts+1,not_before=now()+make_interval(secs=>$4),leased_until=NULL,lease_owner=NULL WHERE lease_owner=$1 AND run_id=$2::text::uuid AND job_id=$3", &[&owner,&run_text(run_id),&job_id.0,&delay.as_secs_f64()]).await.map_err(db)?; + } + Ok(()) + } + pub(crate) async fn clear_check_run_update_id( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + expected: u64, + ) -> Result<(), ControlError> { + let client = self.writer().await?; + client.execute("UPDATE check_run_updates SET check_run_id=NULL WHERE lease_owner=$1 AND run_id=$2::text::uuid AND job_id=$3 AND check_run_id=$4", &[&owner,&run_text(run_id),&job_id.0,&(expected as i64)]).await.map(|_|()).map_err(db) + } + + /// Release a lease without counting an attempt: the sender is waiting out + /// a rate limit, not failing to deliver. + pub(crate) async fn defer_check_run_update( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + delay: std::time::Duration, + ) -> Result<(), ControlError> { + let client = self.writer().await?; + client.execute("UPDATE check_run_updates SET not_before=now()+make_interval(secs=>$4),leased_until=NULL,lease_owner=NULL WHERE lease_owner=$1 AND run_id=$2::text::uuid AND job_id=$3", &[&owner,&run_text(run_id),&job_id.0,&delay.as_secs_f64()]).await.map(|_|()).map_err(db) + } + + /// Extend `owner`'s lease on one row. A row another sender re-leased has + /// a different `lease_owner`, so ownership alone decides: `false` means + /// the caller lost the row and must not call GitHub for it. + pub(crate) async fn renew_check_run_update( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + lease_for: std::time::Duration, + ) -> Result { + let client = self.writer().await?; + let renewed = client + .execute( + "UPDATE check_run_updates SET leased_until=now()+make_interval(secs=>$4) \ + WHERE lease_owner=$1 AND run_id=$2::text::uuid AND job_id=$3", + &[ + &owner, + &run_text(run_id), + &job_id.0, + &lease_for.as_secs_f64(), + ], + ) + .await + .map_err(db)?; + Ok(renewed == 1) + } + + /// Append a durable projection wake for one run (or one job) *after* a + /// reporter stamped `reports_check_runs`. + /// + /// Unlike a `JobStatus` event this is never stamped `Stale`, so a job + /// already terminal when the run's reporting flag lands (an `if: false` + /// leg concluded during submit) still reaches the projector. + pub(crate) async fn append_check_run_projection( + &self, + run_id: RunId, + job_id: Option<&JobId>, + ) -> Result<(), ControlError> { + let mut client = self.writer().await?; + let tx = client.transaction().await.map_err(db)?; + let payload = serde_json::json!({"job_id": job_id.map(|job| job.0.clone())}); + super::dispatch::emit_outbox(&tx, Some(run_id), "check_run_projection.v1", payload).await?; + tx.commit().await.map_err(db)?; + Ok(()) + } +} + +async fn read_rows( + tx: &Transaction<'_>, + after: OutboxBookmark, + limit: usize, +) -> Result, ControlError> { + let rows=tx.query("SELECT txid::text,event_id,run_id::text,job_id,version,origin,topic,payload::text FROM outbox_events WHERE (txid,event_id)>($1::text::xid8,$2) AND txid(0) + .parse() + .map_err(ControlError::backend)?, + event_id: r.get(1), + }, + run_id: r + .get::<_, Option>(2) + .map(|v| codec::run_id(&v)) + .transpose()?, + job_id: r.get(3), + version: r.get(4), + origin: r.get(5), + topic: r.get(6), + payload: codec::from_json(r.get::<_, String>(7).as_str())?, + }) + }) + .collect() +} + +async fn project_row(tx: &Transaction<'_>, row: &OutboxRow) -> Result<(), ControlError> { + let Some(run_id) = row.run_id else { + return Ok(()); + }; + let filter = row.job_id.as_deref(); + let sql = if filter.is_some() { + "SELECT r.reports_check_runs,r.repository,COALESCE(ps.effective_sha,s.submission->>'status_check_sha',s.submission->>'sha',r.head_sha),j.job_id,j.status,j.version,COALESCE(js.display_name,j.job_id),j.check_run_id,j.kind FROM runs r JOIN jobs j ON j.run_id=r.run_id LEFT JOIN job_specs js ON js.run_id=j.run_id AND js.job_id=j.job_id LEFT JOIN run_submissions s ON s.run_id=r.run_id LEFT JOIN run_push_states ps ON ps.run_id=r.run_id WHERE r.run_id=$1::text::uuid AND j.job_id=$2" + } else { + "SELECT r.reports_check_runs,r.repository,COALESCE(ps.effective_sha,s.submission->>'status_check_sha',s.submission->>'sha',r.head_sha),j.job_id,j.status,j.version,COALESCE(js.display_name,j.job_id),j.check_run_id,j.kind FROM runs r JOIN jobs j ON j.run_id=r.run_id LEFT JOIN job_specs js ON js.run_id=j.run_id AND js.job_id=j.job_id LEFT JOIN run_submissions s ON s.run_id=r.run_id LEFT JOIN run_push_states ps ON ps.run_id=r.run_id WHERE r.run_id=$1::text::uuid" + }; + let rows = if let Some(job) = filter { + tx.query(sql, &[&run_text(run_id), &job]).await + } else { + tx.query(sql, &[&run_text(run_id)]).await + } + .map_err(db)?; + for r in rows { + let reports: bool = r.get(0); + let kind: String = r.get(8); + if !reports || kind == "matrix_parent" || kind == "reusable_caller" { + continue; + } + let status: String = r.get(4); + let version: i64 = r.get(5); + let payload = serde_json::json!({"repository":r.get::<_,String>(1),"sha":r.get::<_,String>(2),"job_id":r.get::<_,String>(3),"status":status,"name":r.get::<_,String>(6)}); + let id: r#Option = r.get(7); + tx.execute("INSERT INTO check_run_updates(run_id,job_id,installation_id,check_run_id,version,payload,not_before,attempts,leased_until,lease_owner) VALUES($1::text::uuid,$2,0,$3,$4,$5::text::jsonb,now(),0,NULL,NULL) ON CONFLICT(run_id,job_id) DO UPDATE SET payload=EXCLUDED.payload,version=EXCLUDED.version,check_run_id=COALESCE(EXCLUDED.check_run_id,check_run_updates.check_run_id),not_before=now(),leased_until=NULL,lease_owner=NULL,attempts=0 WHERE EXCLUDED.version>check_run_updates.version", &[&run_text(run_id),&r.get::<_,String>(3),&id,&version,&payload.to_string()]).await.map_err(db)?; + } + Ok(()) +} diff --git a/crates/preloop-runner-server/src/control/pg/mod.rs b/crates/preloop-runner-server/src/control/pg/mod.rs index d8cdf7041..fa820b446 100644 --- a/crates/preloop-runner-server/src/control/pg/mod.rs +++ b/crates/preloop-runner-server/src/control/pg/mod.rs @@ -18,6 +18,7 @@ //! - [`lookups`]: request/callback/check-run/run lookups, push state, //! run numbers, key fingerprint, archival and queue statistics. +mod check_runs; mod codec; mod dispatch; mod graph; @@ -39,7 +40,7 @@ use tokio_postgres::{Client, NoTls}; /// The schema this build creates and accepts. Greenfield v1: there are no /// migrations, a database at any other version is refused. -pub(crate) const SCHEMA_VERSION: &str = "4"; +pub(crate) const SCHEMA_VERSION: &str = "5"; /// The agreed schema plus `schema_meta`. const SCHEMA_SQL: &str = include_str!("schema.sql"); diff --git a/crates/preloop-runner-server/src/control/pg/outbox.rs b/crates/preloop-runner-server/src/control/pg/outbox.rs index c5668a51d..e01a35209 100644 --- a/crates/preloop-runner-server/src/control/pg/outbox.rs +++ b/crates/preloop-runner-server/src/control/pg/outbox.rs @@ -119,10 +119,14 @@ impl PgBackend { let client = self.writer().await?; client .execute( - "DELETE FROM outbox_events WHERE (event_id, created_at) IN \ - (SELECT event_id, created_at FROM outbox_events \ - WHERE created_at < now() - make_interval(secs => $1) \ - ORDER BY created_at LIMIT $2)", + "DELETE FROM outbox_events o WHERE (o.event_id,o.created_at) IN \ + (SELECT e.event_id,e.created_at FROM outbox_events e \ + WHERE e.created_at < now() - make_interval(secs => $1) \ + AND (NOT EXISTS (SELECT 1 FROM consumer_offsets) OR \ + (e.txid,e.event_id) <= \ + (SELECT last_txid,last_event_id FROM consumer_offsets \ + ORDER BY last_txid,last_event_id LIMIT 1)) \ + ORDER BY e.created_at LIMIT $2)", &[&older_than.as_secs_f64(), &(limit as i64)], ) .await diff --git a/crates/preloop-runner-server/src/control/pg/schema.sql b/crates/preloop-runner-server/src/control/pg/schema.sql index 915f4e759..5119a5574 100644 --- a/crates/preloop-runner-server/src/control/pg/schema.sql +++ b/crates/preloop-runner-server/src/control/pg/schema.sql @@ -658,12 +658,13 @@ CREATE TABLE check_run_updates ( job_id text NOT NULL, installation_id bigint NOT NULL, check_run_id bigint, + version bigint NOT NULL DEFAULT 0, payload jsonb NOT NULL, not_before timestamptz NOT NULL DEFAULT now(), attempts integer NOT NULL DEFAULT 0, leased_until timestamptz, - PRIMARY KEY (run_id, job_id), - FOREIGN KEY (run_id, job_id) REFERENCES jobs(run_id, job_id) ON DELETE CASCADE + lease_owner text, + PRIMARY KEY (run_id, job_id) ); CREATE INDEX check_run_updates_queue ON check_run_updates(installation_id, not_before); diff --git a/crates/preloop-runner-server/src/control/pg/trait_impl.rs b/crates/preloop-runner-server/src/control/pg/trait_impl.rs index d4b5198d2..792bb098d 100644 --- a/crates/preloop-runner-server/src/control/pg/trait_impl.rs +++ b/crates/preloop-runner-server/src/control/pg/trait_impl.rs @@ -235,6 +235,107 @@ impl ControlBackend for PgBackend { ) -> Result { self.prune_outbox(older_than, limit).await } + async fn consume_check_run_outbox( + &self, + owner: &str, + lease_for: std::time::Duration, + limit: usize, + ) -> Result { + self.consume_check_run_outbox(owner, lease_for, limit).await + } + async fn lease_check_run_updates( + &self, + owner: &str, + lease_for: std::time::Duration, + limit: usize, + ) -> Result, ControlError> { + self.lease_check_run_updates(owner, lease_for, limit).await + } + async fn set_check_run_update_id( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + version: i64, + check_run_id: u64, + ) -> Result<(), ControlError> { + self.set_check_run_update_id(owner, run_id, job_id, version, check_run_id) + .await + } + async fn finish_check_run_update( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + version: i64, + ) -> Result<(), ControlError> { + self.finish_check_run_update(owner, run_id, job_id, version) + .await + } + async fn retry_check_run_update( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + delay: std::time::Duration, + permanent: bool, + ) -> Result<(), ControlError> { + self.retry_check_run_update(owner, run_id, job_id, delay, permanent) + .await + } + async fn clear_check_run_update_id( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + expected: u64, + ) -> Result<(), ControlError> { + self.clear_check_run_update_id(owner, run_id, job_id, expected) + .await + } + async fn enqueue_check_run_update( + &self, + update: CheckRunUpdateInput, + ) -> Result<(), ControlError> { + self.enqueue_check_run_update(update).await + } + async fn defer_check_run_update( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + delay: std::time::Duration, + ) -> Result<(), ControlError> { + self.defer_check_run_update(owner, run_id, job_id, delay) + .await + } + async fn renew_check_run_update( + &self, + owner: &str, + run_id: RunId, + job_id: &JobId, + lease_for: std::time::Duration, + ) -> Result { + self.renew_check_run_update(owner, run_id, job_id, lease_for) + .await + } + async fn append_check_run_projection( + &self, + run_id: RunId, + job_id: Option<&JobId>, + ) -> Result<(), ControlError> { + self.append_check_run_projection(run_id, job_id).await + } + async fn outbox_head(&self) -> Result, ControlError> { + self.outbox_head().await.map(Some) + } + async fn outbox_read( + &self, + after: OutboxBookmark, + limit: usize, + ) -> Result, ControlError> { + self.outbox_read(after, limit).await + } async fn reap_inputs(&self) -> Result { self.reap_inputs().await } diff --git a/crates/preloop-runner-server/src/control/tests.rs b/crates/preloop-runner-server/src/control/tests.rs index 69967b772..0f08806dc 100644 --- a/crates/preloop-runner-server/src/control/tests.rs +++ b/crates/preloop-runner-server/src/control/tests.rs @@ -2663,6 +2663,448 @@ pub(crate) mod suite { ); } + /// Drain the projector until a short batch, returning rows consumed. + async fn project_all_check_run_events(backend: &dyn ControlBackend, owner: &str) -> usize { + let mut total = 0; + for _ in 0..64 { + let n = backend + .consume_check_run_outbox(owner, std::time::Duration::from_secs(30), 256) + .await + .unwrap(); + total += n; + if n < 256 { + break; + } + } + total + } + + async fn lease_check_runs( + backend: &dyn ControlBackend, + owner: &str, + ) -> Vec { + backend + .lease_check_run_updates(owner, std::time::Duration::from_secs(30), 32) + .await + .unwrap() + } + + async fn outbox_len(backend: &dyn ControlBackend) -> usize { + backend + .outbox_read( + OutboxBookmark { + txid: 0, + event_id: 0, + }, + 10_000, + ) + .await + .unwrap() + .len() + } + + /// Consume until the projector has nothing left to read. + /// + /// On a shared Postgres server a concurrent transaction (another test's + /// database, same cluster) holds `pg_snapshot_xmin` back, hiding freshly + /// committed rows for a moment; retry instead of assuming instant + /// visibility. + async fn consume_until_quiescent(backend: &dyn ControlBackend, owner: &str) -> usize { + let mut total = 0; + for _ in 0..600 { + let n = backend + .consume_check_run_outbox(owner, std::time::Duration::from_secs(30), 256) + .await + .unwrap(); + total += n; + if n == 0 && total > 0 { + break; + } + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + } + total + } + + /// Read the outbox length once two consecutive reads agree, so a delayed + /// commit cannot make an assertion count a half-visible stream. + async fn stable_outbox_len(backend: &dyn ControlBackend) -> usize { + let mut last = outbox_len(backend).await; + for _ in 0..200 { + tokio::time::sleep(std::time::Duration::from_millis(5)).await; + let now = outbox_len(backend).await; + if now == last { + return now; + } + last = now; + } + last + } + + /// Wait until the projector has materialized `status` for the job. + async fn wait_for_projected_status( + backend: &dyn ControlBackend, + run_id: RunId, + job_id: &JobId, + status: &str, + ) -> crate::control::types::CheckRunUpdate { + for _ in 0..600 { + let _ = consume_until_quiescent(backend, "suite-projector").await; + let rows = lease_check_runs(backend, "suite-probe").await; + if let Some(row) = rows + .iter() + .find(|row| row.run_id == run_id && row.job_id == *job_id) + && row.payload["status"] == status + { + return row.clone(); + } + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + } + panic!("check-run row for {run_id}/{job_id} never reached {status}"); + } + + /// The desired-state row is latest-version-wins: an older projected event + /// (or a direct insert) never overwrites a newer row, so a late `queued` + /// cannot regress a `success` already in the queue. + pub(crate) async fn check_run_projection_never_regresses_a_newer_row( + backend: &dyn ControlBackend, + ) { + let run_id = RunId::new(); + let build = JobId("build".to_owned()); + backend + .submit_run(submit_run(run_id, vec![submit_job(run_id, "build", 1)])) + .await + .unwrap(); + backend.set_reports_check_runs(run_id, true).await.unwrap(); + + // A newer desired state than any event this run can produce. + backend + .enqueue_check_run_update(CheckRunUpdateInput { + run_id, + job_id: build.clone(), + installation_id: 0, + check_run_id: None, + version: 9_999, + payload: serde_json::json!({ + "repository": "owner/repo", + "sha": "abc123", + "job_id": "build", + "status": "success", + "name": "build", + }), + }) + .await + .unwrap(); + // Direct insert of an older version is a no-op too. + backend + .enqueue_check_run_update(CheckRunUpdateInput { + run_id, + job_id: build.clone(), + installation_id: 0, + check_run_id: None, + version: 3, + payload: serde_json::json!({ + "repository": "owner/repo", + "sha": "abc123", + "job_id": "build", + "status": "queued", + "name": "build", + }), + }) + .await + .unwrap(); + + // A real projection wake for the same job must not regress the row. + backend + .append_check_run_projection(run_id, Some(&build)) + .await + .unwrap(); + project_all_check_run_events(backend, "suite-projector").await; + + let rows = lease_check_runs(backend, "suite-probe").await; + let row = rows + .iter() + .find(|row| row.run_id == run_id && row.job_id == build) + .expect("the queue holds a row for build"); + assert_eq!( + row.version, 9_999, + "an older projected version must not overwrite a newer row" + ); + assert_eq!(row.payload["status"], "success"); + } + + /// The acquire transition must reach GitHub: claiming a job writes + /// `job.started.v1` and moves the row to in_progress, the projector + /// coalesces that into an in_progress queue row, and the sender then + /// PATCHes it (exercised end-to-end by the github.rs sender tests). + pub(crate) async fn acquire_projects_in_progress_check_run(backend: &dyn ControlBackend) { + let run_id = RunId::new(); + let build = JobId("build".to_owned()); + let runner = backend + .register_runner(register_runner("check-run-r1")) + .await + .unwrap(); + let session = backend + .create_session(create_session(runner.runner.id)) + .await + .unwrap(); + backend + .submit_run(submit_run(run_id, vec![submit_job(run_id, "build", 1)])) + .await + .unwrap(); + backend.set_reports_check_runs(run_id, true).await.unwrap(); + backend + .append_check_run_projection(run_id, Some(&build)) + .await + .unwrap(); + let queued = wait_for_projected_status(backend, run_id, &build, "queued").await; + let queued_version = queued.version; + + let poll = backend + .poll_session(poll(&session.session_id, runner.runner.id)) + .await + .unwrap(); + assert!( + matches!(poll, PollOutcome::Claimed(_)), + "expected a claim, got {poll:?}" + ); + let row = wait_for_projected_status(backend, run_id, &build, "in_progress").await; + assert!( + row.version > queued_version, + "the acquire transition bumps the row version ({} vs {queued_version})", + row.version + ); + } + + /// The consumer bookmark is durable state, not process memory: a second + /// consumer owner (a restarted process) resumes exactly where the first + /// stopped, so events committed while it was down are delivered once. + pub(crate) async fn check_run_bookmark_survives_consumer_restart(backend: &dyn ControlBackend) { + let run_id = RunId::new(); + backend + .submit_run(submit_run(run_id, vec![submit_job(run_id, "build", 1)])) + .await + .unwrap(); + // Postgres reads only below the cluster-wide snapshot `xmin`, so an + // open transaction in another test's database on the shared server + // hides these freshly committed rows for a moment. Retry like the + // consumer loops below instead of counting once. + let mut total = 0; + for _ in 0..600 { + total = outbox_len(backend).await; + if total > 1 { + break; + } + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + } + assert!(total > 1, "a submit emits several outbox rows"); + + // First consumer reads exactly one event and commits the bookmark. + // Its lease is left expired so the restarted consumer can take over + // without waiting out the production lease window. Retry: on a shared + // Postgres cluster a concurrent transaction can hide the row briefly. + let mut first = 0; + for _ in 0..600 { + first = backend + .consume_check_run_outbox( + "suite-consumer-1", + std::time::Duration::from_millis(1), + 1, + ) + .await + .unwrap(); + if first == 1 { + break; + } + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + } + assert_eq!(first, 1, "the first consumer reads one event"); + tokio::time::sleep(std::time::Duration::from_millis(5)).await; + + // Events committed while it was "down" (the second submit) plus the + // remainder are delivered after the restart. Retry until the counts + // settle: a concurrent transaction on a shared Postgres cluster can + // delay visibility of freshly committed rows. + let run_id2 = RunId::new(); + backend + .submit_run(submit_run(run_id2, vec![submit_job(run_id2, "build", 1)])) + .await + .unwrap(); + let mut delivered = 0; + for _ in 0..600 { + delivered += backend + .consume_check_run_outbox( + "suite-consumer-2", + std::time::Duration::from_secs(30), + 256, + ) + .await + .unwrap(); + if delivered > 0 && first + delivered == stable_outbox_len(backend).await { + break; + } + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + } + assert!( + delivered > 0, + "events committed while the consumer was down are delivered after restart" + ); + assert_eq!( + first + delivered, + stable_outbox_len(backend).await, + "a restarted consumer must resume at the persisted bookmark, not replay the stream" + ); + } + + /// Renewal keeps only the current owner's lease: a sender whose lease + /// expired and was re-leased by another sender is told so (and so must + /// not POST), and its renewal attempt does not steal the row back. + pub(crate) async fn check_run_lease_renewal_respects_takeover(backend: &dyn ControlBackend) { + let run_id = RunId::new(); + let build = JobId("build".to_owned()); + backend + .submit_run(submit_run(run_id, vec![submit_job(run_id, "build", 1)])) + .await + .unwrap(); + backend + .enqueue_check_run_update(CheckRunUpdateInput { + run_id, + job_id: build.clone(), + installation_id: 0, + check_run_id: None, + version: 1, + payload: serde_json::json!({ + "repository": "owner/repo", + "sha": "abc123", + "status": "queued", + "name": "build", + }), + }) + .await + .unwrap(); + let holds = |rows: &[crate::control::types::CheckRunUpdate]| { + rows.iter() + .any(|row| row.run_id == run_id && row.job_id == build) + }; + + let first = backend + .lease_check_run_updates("sender-a", std::time::Duration::from_millis(1), 100) + .await + .unwrap(); + assert!(holds(&first), "sender-a leases the row"); + tokio::time::sleep(std::time::Duration::from_millis(20)).await; + let second = backend + .lease_check_run_updates("sender-b", std::time::Duration::from_secs(30), 100) + .await + .unwrap(); + assert!(holds(&second), "sender-b re-leases the expired row"); + + let lease = std::time::Duration::from_secs(60); + assert!( + !backend + .renew_check_run_update("sender-a", run_id, &build, lease) + .await + .unwrap(), + "the superseded sender must learn it lost the row" + ); + assert!( + backend + .renew_check_run_update("sender-b", run_id, &build, lease) + .await + .unwrap(), + "the current owner renews" + ); + let third = backend + .lease_check_run_updates("sender-c", std::time::Duration::from_secs(30), 100) + .await + .unwrap(); + assert!( + !holds(&third), + "a renewed lease is not up for grabs, and the failed renewal took nothing" + ); + } + + /// Outbox prune never passes the durable consumer bookmark, and still + /// bounds growth by age when no consumer has registered. + pub(crate) async fn prune_outbox_respects_slowest_consumer(backend: &dyn ControlBackend) { + // No consumer: age alone bounds the stream. + let run_id = RunId::new(); + backend + .submit_run(submit_run(run_id, vec![submit_job(run_id, "build", 1)])) + .await + .unwrap(); + tokio::time::sleep(std::time::Duration::from_millis(5)).await; + let mut before = 0; + for _ in 0..600 { + before = stable_outbox_len(backend).await; + if before > 0 { + break; + } + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + } + assert!(before > 0, "committed outbox rows become visible"); + let pruned = backend + .prune_outbox(std::time::Duration::ZERO, 10_000) + .await + .unwrap(); + assert!( + pruned >= 1, + "with no durable consumer, age prunes the stream" + ); + // Postgres hides rows above the cluster-wide snapshot `xmin`, so a + // row the count missed can surface after the prune; rows never + // vanish except by pruning. Hence `>=`: prune removed no more than it + // reported. + assert!( + outbox_len(backend).await >= before - pruned as usize, + "prune removes no more rows than it counted" + ); + + // A consumer's bookmark: rows at/below it are prunable, rows past it + // are retained however old they are. + let run_id = RunId::new(); + backend + .submit_run(submit_run(run_id, vec![submit_job(run_id, "build", 1)])) + .await + .unwrap(); + let fast = consume_until_quiescent(backend, "prune-consumer-fast").await; + assert!(fast > 0); + // Rows past the bookmark must be visible before the prune, or the + // "they survive" assertion below is vacuous: wait until the second + // submit's rows are counted, not just the first run's. + let before_second = stable_outbox_len(backend).await; + let run_id2 = RunId::new(); + backend + .submit_run(submit_run(run_id2, vec![submit_job(run_id2, "build", 1)])) + .await + .unwrap(); + let mut total = 0; + for _ in 0..600 { + total = stable_outbox_len(backend).await; + if total > before_second { + break; + } + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + } + assert!( + total > before_second, + "the second submit's rows become visible" + ); + let pruned = backend + .prune_outbox(std::time::Duration::ZERO, 10_000) + .await + .unwrap(); + assert!(pruned >= 1, "rows at or below the bookmark are prunable"); + let remaining = outbox_len(backend).await; + assert!( + remaining >= total - pruned as usize, + "prune removes no more rows than it counted" + ); + assert!( + remaining >= 1, + "rows past the consumer bookmark must survive an age prune" + ); + } + /// `run_dispatch_info` flags the expandable placeholder nodes — a /// deferred-matrix parent and a reusable caller. They never dispatch: /// expansion replaces them with the legs that mint their own checks, so @@ -5051,6 +5493,121 @@ mod pg { suite::prune_outbox_keeps_fresh_rows_and_bounds_a_batch(&backend).await; } + #[tokio::test] + async fn check_run_projection_never_regresses_a_newer_row() { + let Some((_pg, backend)) = backend().await else { + return skip_no_postgres(); + }; + suite::check_run_projection_never_regresses_a_newer_row(&backend).await; + } + + #[tokio::test] + async fn acquire_projects_in_progress_check_run() { + let Some((_pg, backend)) = backend().await else { + return skip_no_postgres(); + }; + suite::acquire_projects_in_progress_check_run(&backend).await; + } + + #[tokio::test] + async fn check_run_bookmark_survives_consumer_restart() { + let Some((_pg, backend)) = backend().await else { + return skip_no_postgres(); + }; + suite::check_run_bookmark_survives_consumer_restart(&backend).await; + } + + #[tokio::test] + async fn check_run_lease_renewal_respects_takeover() { + let Some((_pg, backend)) = backend().await else { + return skip_no_postgres(); + }; + suite::check_run_lease_renewal_respects_takeover(&backend).await; + } + + #[tokio::test] + async fn prune_outbox_respects_slowest_consumer() { + let Some((_pg, backend)) = backend().await else { + return skip_no_postgres(); + }; + suite::prune_outbox_respects_slowest_consumer(&backend).await; + } + + /// The consumer bookmark is row state in `consumer_offsets`, so a second + /// connection (another node, or the process after a restart) resumes at + /// it instead of replaying the stream. + #[tokio::test] + async fn check_run_bookmark_survives_a_second_connection() { + let Some((_pg, first, second)) = backend_pair().await else { + return skip_no_postgres(); + }; + let run_id = RunId::new(); + first + .submit_run(submit_run(run_id, vec![submit_job(run_id, "build", 1)])) + .await + .unwrap(); + let mut consumed = 0; + for _ in 0..600 { + consumed = first + .consume_check_run_outbox("pg-consumer-1", std::time::Duration::from_millis(1), 256) + .await + .unwrap(); + if consumed > 0 { + break; + } + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + } + assert!(consumed > 0, "the first connection reads the run's events"); + tokio::time::sleep(std::time::Duration::from_millis(5)).await; + + let run_id2 = RunId::new(); + second + .submit_run(submit_run(run_id2, vec![submit_job(run_id2, "build", 1)])) + .await + .unwrap(); + // Retry until the counts settle: a concurrent transaction on the + // shared test cluster can hold `pg_snapshot_xmin` back. + let mut delivered = 0; + for _ in 0..600 { + delivered += second + .consume_check_run_outbox("pg-consumer-2", std::time::Duration::from_secs(30), 256) + .await + .unwrap(); + let total = second + .outbox_read( + OutboxBookmark { + txid: 0, + event_id: 0, + }, + 10_000, + ) + .await + .unwrap() + .len(); + if delivered > 0 && consumed + delivered == total { + break; + } + tokio::time::sleep(std::time::Duration::from_millis(10)).await; + } + assert!(delivered > 0, "the down-window events are delivered"); + let total = second + .outbox_read( + OutboxBookmark { + txid: 0, + event_id: 0, + }, + 10_000, + ) + .await + .unwrap() + .len(); + assert_eq!( + consumed + delivered, + total, + "a second connection must resume at the persisted bookmark (no replay, no skip)" + ); + } + #[tokio::test] async fn sessionless_runner_is_not_idle_capacity() { let Some((_pg, backend)) = backend().await else { @@ -6099,6 +6656,90 @@ mod lite { .await; } + #[tokio::test] + async fn check_run_projection_never_regresses_a_newer_row() { + suite::check_run_projection_never_regresses_a_newer_row(&LiteBackend::in_memory().unwrap()) + .await; + } + + #[tokio::test] + async fn acquire_projects_in_progress_check_run() { + suite::acquire_projects_in_progress_check_run(&LiteBackend::in_memory().unwrap()).await; + } + + #[tokio::test] + async fn check_run_bookmark_survives_consumer_restart() { + suite::check_run_bookmark_survives_consumer_restart(&LiteBackend::in_memory().unwrap()) + .await; + } + + #[tokio::test] + async fn prune_outbox_respects_slowest_consumer() { + suite::prune_outbox_respects_slowest_consumer(&LiteBackend::in_memory().unwrap()).await; + } + + #[tokio::test] + async fn check_run_lease_renewal_respects_takeover() { + suite::check_run_lease_renewal_respects_takeover(&LiteBackend::in_memory().unwrap()).await; + } + + /// The bookmark lives in the SQLite file: reopen the database and only the + /// events committed while the consumer was down are delivered. + #[tokio::test] + async fn check_run_bookmark_survives_a_file_reopen() { + let temp = tempfile::tempdir().unwrap(); + let path = temp.path().join("preloop.db"); + let run_id = RunId::new(); + let consumed = { + let backend = + LiteBackend::open(&path, false, false, std::time::Duration::from_secs(300)) + .unwrap(); + backend + .submit_run(submit_run(run_id, vec![submit_job(run_id, "build", 1)])) + .await + .unwrap(); + backend + .consume_check_run_outbox( + "reopen-consumer", + std::time::Duration::from_millis(1), + 256, + ) + .await + .unwrap() + }; + assert!(consumed > 0); + tokio::time::sleep(std::time::Duration::from_millis(5)).await; + + // Down window: a fresh handle appends without consuming. + let reopened = + LiteBackend::open(&path, false, false, std::time::Duration::from_secs(300)).unwrap(); + let run_id2 = RunId::new(); + reopened + .submit_run(submit_run(run_id2, vec![submit_job(run_id2, "build", 1)])) + .await + .unwrap(); + let total = reopened + .outbox_read( + crate::control::types::OutboxBookmark { + txid: 0, + event_id: 0, + }, + 10_000, + ) + .await + .unwrap() + .len(); + let delivered = reopened + .consume_check_run_outbox("reopen-consumer-2", std::time::Duration::from_secs(30), 256) + .await + .unwrap(); + assert_eq!( + delivered, + total - consumed, + "a reopened consumer must resume at the persisted bookmark" + ); + } + #[tokio::test] async fn sessionless_runner_is_not_idle_capacity() { suite::sessionless_runner_is_not_idle_capacity(&LiteBackend::in_memory().unwrap()).await; diff --git a/crates/preloop-runner-server/src/control/types.rs b/crates/preloop-runner-server/src/control/types.rs index c7483ab2a..84d78a050 100644 --- a/crates/preloop-runner-server/src/control/types.rs +++ b/crates/preloop-runner-server/src/control/types.rs @@ -216,14 +216,15 @@ pub(crate) fn status_parse(s: &str) -> ExecutionStatus { } } -/// A position in the outbox. Rows are read in `(txid, event_id)` order. +/// A position in the transactional outbox. PostgreSQL uses `(txid,event_id)`; +/// SQLite leaves `txid` at zero and orders by `event_id`. #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] pub(crate) struct OutboxBookmark { pub(crate) txid: u64, pub(crate) event_id: i64, } -/// One `outbox_events` row, as the stream consumer reads it. +/// One `outbox_events` row, as a stream consumer reads it. #[derive(Debug, Clone)] pub(crate) struct OutboxRow { pub(crate) bookmark: OutboxBookmark, @@ -238,6 +239,29 @@ pub(crate) struct OutboxRow { pub(crate) payload: serde_json::Value, } +/// A coalesced desired check-run state leased by the background sender. +#[derive(Debug, Clone)] +pub(crate) struct CheckRunUpdate { + pub(crate) run_id: RunId, + pub(crate) job_id: JobId, + pub(crate) installation_id: i64, + pub(crate) check_run_id: Option, + pub(crate) version: i64, + pub(crate) payload: serde_json::Value, + pub(crate) attempts: i32, +} + +/// Backend-neutral durable update for one desired check-run state. +#[derive(Debug, Clone)] +pub(crate) struct CheckRunUpdateInput { + pub(crate) run_id: RunId, + pub(crate) job_id: JobId, + pub(crate) installation_id: i64, + pub(crate) check_run_id: Option, + pub(crate) version: i64, + pub(crate) payload: serde_json::Value, +} + /// Everything the submit path pre-built outside the transaction: the run /// record plus one queue entry per non-skipped job and the correlation /// records minted for dispatchable jobs. diff --git a/crates/preloop-runner-server/src/event_feed.rs b/crates/preloop-runner-server/src/event_feed.rs index 9f9fc0df5..afc68c059 100644 --- a/crates/preloop-runner-server/src/event_feed.rs +++ b/crates/preloop-runner-server/src/event_feed.rs @@ -51,6 +51,7 @@ pub(crate) fn spawn( events: broadcast::Sender, dirty: Arc, ) { + crate::check_run_outbox::spawn_consumer(backend, Arc::clone(&dirty)); let Some(origin) = backend.event_origin().map(str::to_owned) else { return; }; diff --git a/crates/preloop-runner-server/src/github.rs b/crates/preloop-runner-server/src/github.rs index 1987e48bf..cca612858 100644 --- a/crates/preloop-runner-server/src/github.rs +++ b/crates/preloop-runner-server/src/github.rs @@ -226,12 +226,35 @@ async fn send_github_check_request( ) .await?; + // Honour the primary budget advertised on the response: once remaining + // hits zero, wait for the reset instead of spending the next call on a + // guaranteed 403. + breaker.observe_rate_budget(res.headers()); + if !res.status().is_success() { let status = res.status(); + let retry_after = res + .headers() + .get("retry-after") + .and_then(|v| v.to_str().ok()) + .unwrap_or("") + .to_owned(); + let header = |name: &str| { + res.headers() + .get(name) + .and_then(|v| v.to_str().ok()) + .unwrap_or("") + .to_owned() + }; + // GitHub sends `x-ratelimit-reset` on every response, so the reset is + // only meaningful together with `remaining`: `check_retry_delay` + // waits for the reset only when the budget is actually spent. + let reset = header("x-ratelimit-reset"); + let remaining = header("x-ratelimit-remaining"); let err_text = res.text().await.unwrap_or_default(); record_check_reporting(shared, false); return Err(anyhow::anyhow!( - "GitHub Check API failed with status {}: {}", + "GitHub Check API failed with status {}: {}; retry_after={retry_after}; rate_remaining={remaining}; rate_reset={reset}", status, err_text )); @@ -242,56 +265,6 @@ async fn send_github_check_request( Ok(val) } -/// GitHub can briefly return 404 when a PATCH races replication of a newly -/// created check run. Retry only that response; every other error is final. -async fn send_github_check_completion( - shared: &Arc, - token: &str, - repo: &str, - path: &str, - body: &Value, -) -> anyhow::Result { - const RETRY_DELAYS: [Duration; 3] = [ - Duration::from_millis(250), - Duration::from_millis(750), - Duration::from_millis(1_500), - ]; - for (attempt, delay) in RETRY_DELAYS.into_iter().enumerate() { - match send_github_check_request( - shared, - &shared.state.github_lifecycle_breaker, - token, - repo, - reqwest::Method::PATCH, - path, - body, - ) - .await - { - Err(error) if is_check_run_not_found(&error) => { - warn!( - attempt = attempt + 1, - delay_ms = delay.as_millis(), - %error, - "new GitHub check run is not visible to PATCH yet; retrying" - ); - tokio::time::sleep(delay).await; - } - result => return result, - } - } - send_github_check_request( - shared, - &shared.state.github_lifecycle_breaker, - token, - repo, - reqwest::Method::PATCH, - path, - body, - ) - .await -} - pub fn run_details_url(run_id: RunId) -> Option { std::env::var("PRELOOP_PUBLIC_URL") .ok() @@ -306,77 +279,35 @@ pub fn run_details_url(run_id: RunId) -> Option { /// check runs too". pub async fn report_check_run_queued( shared: &Arc, - repo: &str, - sha: &str, + _repo: &str, + _sha: &str, job_id: &JobId, run_id: RunId, ) -> anyhow::Result> { - // Every intake path that reports checks funnels through here; persist the - // flag so a job materialized later (runtime-expanded leg, reusable callee) - // still mints checks, including after a control-plane restart. shared .state .backend .set_reports_check_runs(run_id, true) .await .map_err(|e| anyhow::anyhow!("{e:?}"))?; - let existing_check_run_id = shared + // The state transition already normally emitted a durable event. Intake + // paths such as rerun/push can report a queued check without changing the + // scheduler row, and a job already terminal when the flag lands would + // stamp a `JobStatus` event `Stale`; append an unconditional projection + // wake instead. + shared .state .backend - .job_check_run_id(run_id, job_id) + .append_check_run_projection(run_id, Some(job_id)) .await .map_err(|e| anyhow::anyhow!("{e:?}"))?; - // The per-run mint lock is still node-local metadata: it only serializes - // concurrent reporters on this node so two of them cannot both observe - // "no mapping" and POST. Fetched before the (slow) token resolution so a - // caller that blocks on it does not re-resolve the token afterwards. - let mint_lock = { - let mut inner = shared.state.inner.lock().await; - inner - .check_run_mint_locks - .entry(run_id) - .or_default() - .clone() - }; - let token = resolve_check_run_token(shared, repo).await; - - if let Some(check_run_id) = existing_check_run_id { - match report_existing_check_run_queued(shared, repo, job_id, run_id, check_run_id).await { - Ok(()) => return Ok(Some(check_run_id)), - Err(error) if !is_check_run_not_found(&error) => return Err(error), - Err(error) => { - warn!( - %run_id, - %job_id, - check_run_id, - %error, - "persisted GitHub check run is stale; reconciling it" - ); - shared - .state - .backend - .clear_job_check_run(run_id, job_id, check_run_id) - .await - .map_err(|e| anyhow::anyhow!("{e:?}"))?; - } - } - } - - // Mint under the per-run lock and re-check the mapping inside it: two - // reporters (e.g. an expansion mint and a claim-time in-progress report) - // can both see "no mapping" and POST, and GitHub accepts duplicate check - // runs for the same name+SHA — the loser would strand a `queued` check. - let _guard = mint_lock.lock().await; - let existing = shared + shared.state.events_dirty.notify_one(); + shared .state .backend .job_check_run_id(run_id, job_id) .await - .map_err(|e| anyhow::anyhow!("{e:?}"))?; - if let Some(check_run_id) = existing { - return Ok(Some(check_run_id)); - } - mint_check_run(shared, repo, sha, job_id, run_id, token.as_deref()).await + .map_err(|e| anyhow::anyhow!("{e:?}")) } /// The repository + SHA check runs for this run attach to, when the run @@ -396,229 +327,803 @@ fn check_run_report_coords(run: &crate::models::RunRecord) -> Option<(String, St Some((run.submission.repository.clone(), sha)) } -/// Return the job's check run id, minting a `queued` check when the job was -/// materialized after intake reported (runtime-expanded matrix legs, -/// reusable callee jobs). `None` when the run does not report checks or the -/// mint fails — callers then behave as before this fix: skip the report. +/// Return an already persisted job→check-run mapping, if any. +/// +/// The sender is solely responsible for creating a missing GitHub check run; +/// request handlers never mint one. Callers that need a check for a +/// late-materialized job call [`report_check_run_queued`], which stamps +/// `reports_check_runs` and appends a durable projection wake. pub async fn ensure_check_run_mapped( shared: &Arc, run_id: RunId, job_id: &JobId, ) -> Option { - let run = shared.state.backend.run_record(run_id).await.ok()?; - if let Some(id) = run.job_check_run_ids.get(job_id).copied() { - return Some(id); - } - let coords = check_run_report_coords(&run)?; - let mint_lock = { - let mut inner = shared.state.inner.lock().await; - inner - .check_run_mint_locks - .entry(run_id) - .or_default() - .clone() - }; - let _guard = mint_lock.lock().await; - let existing = shared + shared .state .backend .job_check_run_id(run_id, job_id) .await .ok() - .flatten(); - if let Some(id) = existing { - return Some(id); - } - let (repo, sha) = coords; - let token = resolve_check_run_token(shared, &repo).await; - match mint_check_run(shared, &repo, &sha, job_id, run_id, token.as_deref()).await { - Ok(id) => id, - Err(error) => { - warn!(%run_id, %job_id, ?error, "failed to mint check run for materialized job"); - None - } + .flatten() +} + +fn is_check_run_not_found(error: &anyhow::Error) -> bool { + error.to_string().contains("status 404") +} + +/// Record a rerequest's desired queued state; the sender performs the PATCH. +pub async fn report_existing_check_run_queued( + shared: &Arc, + _repo: &str, + job_id: &JobId, + run_id: RunId, + check_run_id: u64, +) -> anyhow::Result<()> { + shared + .state + .backend + .set_job_check_run(run_id, job_id, check_run_id) + .await + .map_err(|e| anyhow::anyhow!("{e:?}"))?; + shared + .state + .backend + .append_event(&NdjsonEvent::JobStatus { + run_id, + job_id: job_id.clone(), + status: ExecutionStatus::Queued, + reason: None, + }) + .await + .map_err(|e| anyhow::anyhow!("{e:?}"))?; + // The status event above is dropped when the job row settled differently; + // the wake still delivers whatever state the row holds. + shared + .state + .backend + .append_check_run_projection(run_id, Some(job_id)) + .await + .map_err(|e| anyhow::anyhow!("{e:?}"))?; + shared.state.events_dirty.notify_one(); + Ok(()) +} +/// The `external_id` this engine stamps on every check run it creates. It +/// identifies the queue row's own check, so crash-after-POST reconciliation +/// never adopts another workflow's (or another app's) same-named check. +/// +/// Job checks key on `{run_id}:{job_id}`. A workflow-evaluation failure gets a +/// fresh `RunId` on every redelivery but a stable +/// `__workflow_failure__:{name}` job id, so the job id alone is its key. +fn check_run_external_id(update: &crate::control::types::CheckRunUpdate) -> String { + if update.payload.get("kind").and_then(Value::as_str) == Some("workflow_failure") { + update.job_id.0.clone() + } else { + format!("{}:{}", update.run_id, update.job_id) } } -/// POST a `queued` check run (or mint a mock id when no token resolves) and -/// record the job→check-run mapping. Caller holds the per-run mint lock. -async fn mint_check_run( +/// Find the check run this queue row already created on `sha`, by its +/// `external_id`. Pages through every check run on the commit (100 per page, +/// bounded) rather than trusting GitHub's default first page of 30, and asks +/// for `filter=all` so a newer same-named check cannot hide ours. +async fn find_existing_check_run( shared: &Arc, + breaker: &crate::github_breaker::GithubBreaker, + token: &str, repo: &str, sha: &str, - job_id: &JobId, - run_id: RunId, - token: Option<&str>, + external_id: &str, ) -> anyhow::Result> { - let check_run_id = if let Some(token) = token { - let details_url = run_details_url(run_id); + const PER_PAGE: u64 = 100; + const MAX_PAGES: u64 = 10; + for page in 1..=MAX_PAGES { + let payload = match send_github_check_request( + shared, + breaker, + token, + repo, + reqwest::Method::GET, + &format!("commits/{sha}/check-runs?filter=all&per_page={PER_PAGE}&page={page}"), + &Value::Null, + ) + .await + { + Ok(payload) => payload, + Err(error) if is_check_run_not_found(&error) => return Ok(None), + Err(error) => return Err(error), + }; + let check_runs = payload + .get("check_runs") + .and_then(Value::as_array) + .map(Vec::as_slice) + .unwrap_or_default(); + let found = check_runs.iter().find_map(|check_run| { + (check_run.get("external_id").and_then(Value::as_str) == Some(external_id) + && check_run.get("head_sha").and_then(Value::as_str) == Some(sha)) + .then(|| check_run.get("id").and_then(Value::as_u64)) + .flatten() + }); + if found.is_some() { + return Ok(found); + } + let total = payload + .get("total_count") + .and_then(Value::as_u64) + .unwrap_or(0); + if (check_runs.len() as u64) < PER_PAGE || page * PER_PAGE >= total { + return Ok(None); + } + } + Ok(None) +} + +/// Per-installation rate budget key. +/// +/// Installation ids come from [`crate::github_app::installation_for_repo`]; +/// the PAT fallback shares one bucket because it is one token. `0` is never a +/// valid GitHub installation id, so it is never used as a key. +#[derive(Clone, PartialEq, Eq, Hash, Debug)] +pub(crate) enum CheckRunRateKey { + Installation(u64), + Pat, +} + +/// Ceiling on how long a rate limit may park a row; a bogus header must not +/// strand a check run for a day. +const MAX_DEFER: std::time::Duration = std::time::Duration::from_secs(3_600); + +/// How far each renewal extends a row's lease. Every write to GitHub renews +/// first, and this covers one request at the shared client's 30 s timeout +/// with margin, so a row cannot expire under an in-flight POST and be +/// re-leased by another sender that would POST a second check run. +const CHECK_RUN_LEASE_RENEWAL: std::time::Duration = std::time::Duration::from_secs(60); + +/// Error marker for a row another sender re-leased mid-delivery. +const LOST_CHECK_RUN_LEASE: &str = "check-run lease lost to another sender"; + +/// Attempts during which a PATCH 404 keeps the stored check-run id. GitHub +/// can answer 404 while a just-created check run replicates; only a 404 that +/// outlives these backoff retries means the check is really gone, and only +/// then is the id dropped so the next attempt reconciles or re-creates it. +const CHECK_RUN_NOT_FOUND_RETRIES: i32 = 3; + +/// The single background sender for the durable check-run queue. +/// +/// The serve bootstrap spawns one per node; tests build their own and drive +/// [`CheckRunSender::drain_once`]. Rate budgets are per installation, so a +/// throttled installation parks only its own rows and the rest keep flowing. +pub(crate) struct CheckRunSender { + owner: String, + budget: parking_lot::Mutex< + std::collections::HashMap>, + >, +} + +impl Default for CheckRunSender { + fn default() -> Self { + Self::new() + } +} + +impl CheckRunSender { + pub(crate) fn new() -> Self { + Self { + owner: uuid::Uuid::new_v4().to_string(), + budget: parking_lot::Mutex::new(std::collections::HashMap::new()), + } + } + + fn breaker(&self, key: &CheckRunRateKey) -> Arc { + let mut budget = self.budget.lock(); + budget + .entry(key.clone()) + .or_insert_with(|| Arc::new(crate::github_breaker::GithubBreaker::default())) + .clone() + } - let job_name = shared + /// Lease and process one batch. Returns how many rows were leased; the + /// caller keeps looping while batches arrive and sleeps when none do. + pub(crate) async fn drain_once(&self, shared: &Arc) -> usize { + const BATCH: usize = 32; + const LEASE: std::time::Duration = std::time::Duration::from_secs(30); + let updates = match shared .state .backend - .job_display_name(run_id, job_id) + .lease_check_run_updates(&self.owner, LEASE, BATCH) .await - .map_err(|e| anyhow::anyhow!("{e:?}"))? - .unwrap_or_else(|| job_id.0.clone()); - let mut body = serde_json::json!({ - "name": job_name, - "head_sha": sha, - "status": "queued", - "output": { - "title": job_name, - "summary": format!("Waiting for a preloop runner.\n\njob_id: `{}`", job_id.0) + { + Ok(updates) => updates, + Err(error) => { + warn!(?error, "check-run sender lease failed"); + return 0; } - }); - if let Some(url) = details_url { - body["details_url"] = serde_json::json!(url); + }; + let leased = updates.len(); + for update in updates { + self.send_one(shared, &update).await; } + leased + } - match send_github_check_request( - shared, - &shared.state.github_breaker, - token, - repo, - reqwest::Method::POST, - "check-runs", - &body, - ) - .await - { - Ok(response) => match response.get("id").and_then(Value::as_u64) { - Some(id) => { - info!( - %run_id, - %job_id, - check_run_id = id, - "GitHub check run created successfully" - ); - id - } - None => find_existing_check_run(shared, token, repo, sha, &job_id.to_string()) - .await? - .ok_or_else(|| anyhow::anyhow!("GitHub check-run POST returned no id"))?, - }, + async fn send_one( + &self, + shared: &Arc, + update: &crate::control::types::CheckRunUpdate, + ) { + let repo = update + .payload + .get("repository") + .and_then(Value::as_str) + .unwrap_or_default() + .to_owned(); + // Resolve the real installation before budgeting or sending: the + // queue row's `installation_id` is projection metadata (0 = unknown), + // never an authority. + let key = resolve_check_run_rate_key(shared, &repo).await; + let breaker = self.breaker(&key); + if let Some(remaining) = breaker.retry_after() { + // Waiting out a rate limit is not a delivery failure: release the + // row due at GitHub's advertised resume time without burning an + // attempt, so a long throttle cannot exhaust the permanent-drop + // budget. + if let Err(error) = shared + .state + .backend + .defer_check_run_update( + &self.owner, + update.run_id, + &update.job_id, + remaining.min(MAX_DEFER), + ) + .await + { + warn!(run_id=%update.run_id, job_id=%update.job_id, ?error, "failed to defer rate-limited check-run row"); + } + return; + } + let token = resolve_check_run_token(shared, &repo).await; + match send_queued_check_run(shared, &self.owner, &breaker, token.as_deref(), update).await { + Ok(()) => {} + Err(error) if error.to_string().contains(LOST_CHECK_RUN_LEASE) => { + // Another sender owns the row now; it delivers the update. + // Touching the row here would overwrite its lease state. + debug!(run_id=%update.run_id, job_id=%update.job_id, "check-run lease taken over by another sender; skipping"); + } + Err(error) if error.to_string().contains("circuit breaker is open") => { + // A breaker that opened between the budget check and the call + // is still a deferral, not a failure. + let remaining = breaker + .retry_after() + .unwrap_or(std::time::Duration::from_secs(30)); + let _ = shared + .state + .backend + .defer_check_run_update( + &self.owner, + update.run_id, + &update.job_id, + remaining.min(MAX_DEFER), + ) + .await; + } Err(error) => { - // A transport error is ambiguous: GitHub may have - // committed the POST before the connection failed. - match find_existing_check_run(shared, token, repo, sha, &job_id.to_string()).await { - Ok(Some(id)) => id, - Ok(None) => return Err(error), - Err(reconcile_error) => { - return Err(anyhow::anyhow!( - "{error}; check-run reconciliation failed: {reconcile_error}" - )); - } + let permanent = update.attempts >= 7 || error.to_string().contains("status 422"); + let delay = check_retry_delay(&error, update.attempts); + warn!(run_id=%update.run_id, job_id=%update.job_id, attempts=update.attempts, permanent, ?error, "check-run sender attempt failed"); + if let Err(store_error) = shared + .state + .backend + .retry_check_run_update( + &self.owner, + update.run_id, + &update.job_id, + delay, + permanent, + ) + .await + { + warn!(run_id=%update.run_id, job_id=%update.job_id, ?store_error, "failed to persist check-run retry state"); } } } - } else { - info!(%run_id, %job_id, "GitHub token not configured, using mock check run"); - rand::random::() as u64 - }; + } +} - let mapping_changed = shared - .state - .backend - .set_job_check_run(run_id, job_id, check_run_id) - .await - .map_err(|e| anyhow::anyhow!("{e:?}"))?; - if mapping_changed { - // The mapping is meaningful while the run lives, and the next status - // event may be hours away. `set_job_check_run` persisted the event - // inside its own transaction; only the broadcast remains. - shared - .state - .emit_persisted(preloop_gha_protocol::NdjsonEvent::CheckRunCreated { run_id }) - .await; +/// The installation (or PAT) whose budget governs this repository's check +/// runs. Resolution failure falls back to the PAT bucket; it never invents an +/// installation id. +async fn resolve_check_run_rate_key(shared: &Arc, repo: &str) -> CheckRunRateKey { + match crate::github_app::select_app_for_repo(shared, repo).await { + Some(app) => match crate::github_app::installation_for_repo(&app, repo).await { + Ok(installation) if installation != 0 => CheckRunRateKey::Installation(installation), + _ => CheckRunRateKey::Pat, + }, + None => CheckRunRateKey::Pat, } - Ok(Some(check_run_id)) } -fn is_check_run_not_found(error: &anyhow::Error) -> bool { - error.to_string().contains("status 404") +/// Drain the durable check-run queue. This is the only path that invokes the +/// GitHub Checks API; request handlers only append desired-state events. +pub(crate) async fn run_check_run_sender(shared: Arc) { + let sender = CheckRunSender::new(); + while !shared.shutdown.is_cancelled() { + let leased = sender.drain_once(&shared).await; + if leased == 0 { + tokio::select! { + _ = shared.shutdown.cancelled() => break, + _ = tokio::time::sleep(std::time::Duration::from_millis(250)) => {}, + } + } + } } -/// Move an existing GitHub check run back to the queue after a rerequest. -pub async fn report_existing_check_run_queued( +/// Test-only single pass of the check-run sender, mirroring the bootstrap +/// loop. Tests that assert a check-run mapping was recorded must drain the +/// sender: the sender is the only path that writes one, so a webhook alone +/// leaves `job_check_run_ids` empty. +#[cfg(any(test, feature = "test-support"))] +pub async fn drain_check_run_sender(shared: &Arc) -> usize { + CheckRunSender::new().drain_once(shared).await +} + +fn check_retry_delay(error: &anyhow::Error, attempts: i32) -> std::time::Duration { + let text = error.to_string(); + if let Some(value) = text + .split("retry_after=") + .nth(1) + .and_then(|v| v.split(';').next()) + && let Ok(seconds) = value.trim().parse::() + && seconds > 0 + { + return std::time::Duration::from_secs(seconds.min(86_400)); + } + // `rate_reset` is `x-ratelimit-reset`: an absolute unix timestamp. GitHub + // sends it on every response (404, 422, 5xx included), so it only governs + // the retry when the primary budget is actually spent; otherwise a + // transient failure would wait out the rest of the rate window. + let budget_spent = text + .split("rate_remaining=") + .nth(1) + .and_then(|v| v.split(|c: char| !c.is_ascii_digit()).next()) + .and_then(|v| v.parse::().ok()) + == Some(0); + if budget_spent + && let Some(value) = text + .split("rate_reset=") + .nth(1) + .and_then(|v| v.split(|c: char| !c.is_ascii_digit()).next()) + && let Ok(reset) = value.parse::() + { + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|since| since.as_secs()) + .unwrap_or(0); + if reset > now { + return std::time::Duration::from_secs((reset - now).min(86_400)); + } + } + if text.contains("secondary rate limit") || text.contains("abuse detection") { + return std::time::Duration::from_secs(60); + } + let base = 250_u64.saturating_mul(1_u64 << (attempts.clamp(0, 8) as u32)); + std::time::Duration::from_millis(base + rand::random::() % 250) +} + +/// Render and deliver one queued update. `breaker` is the resolved +/// installation's rate budget; every request this function makes goes +/// through it. +async fn send_queued_check_run( shared: &Arc, - repo: &str, - job_id: &JobId, - run_id: RunId, - check_run_id: u64, + owner: &str, + breaker: &crate::github_breaker::GithubBreaker, + token: Option<&str>, + update: &crate::control::types::CheckRunUpdate, ) -> anyhow::Result<()> { - let token = resolve_check_run_token(shared, repo).await; - if let Some(token) = &token { - let mut body = serde_json::json!({ - "status": "queued", - }); - if let Some(url) = run_details_url(run_id) { - body["details_url"] = serde_json::json!(url); + let repo = update + .payload + .get("repository") + .and_then(Value::as_str) + .ok_or_else(|| anyhow::anyhow!("check-run queue row has no repository"))?; + let sha = update + .payload + .get("sha") + .and_then(Value::as_str) + .ok_or_else(|| anyhow::anyhow!("check-run queue row has no head sha"))?; + let name = update + .payload + .get("name") + .and_then(Value::as_str) + .unwrap_or(update.job_id.0.as_str()); + let status = update + .payload + .get("status") + .and_then(Value::as_str) + .unwrap_or("queued"); + let Some(token) = token else { + info!(run_id=%update.run_id, job_id=%update.job_id, status, "GitHub token not configured; check-run update simulated locally"); + // A token-less engine still records a local check-run id. The base + // minted exactly this mock at submit, before the sender took over + // minting; the mapping is what `job_check_run_ids` serves and what + // must survive a restart, so dropping it would silently erase the + // mock path (`check_run_ids_survive_a_restart_before_any_job_event`). + let existing = shared + .state + .backend + .job_check_run_id(update.run_id, &update.job_id) + .await + .ok() + .flatten(); + if existing.is_none() { + let mock = rand::random::() as u64; + let mapping_changed = shared + .state + .backend + .set_job_check_run(update.run_id, &update.job_id, mock) + .await + .map_err(|e| anyhow::anyhow!("{e:?}"))?; + if mapping_changed { + shared + .state + .emit_persisted(preloop_gha_protocol::NdjsonEvent::CheckRunCreated { + run_id: update.run_id, + }) + .await; + } + } + shared + .state + .backend + .finish_check_run_update(owner, update.run_id, &update.job_id, update.version) + .await + .map_err(|e| anyhow::anyhow!("{e:?}"))?; + return Ok(()); + }; + let mut check_run_id = update.check_run_id; + if check_run_id.is_none() { + // The persisted job mapping is the first dedup source: a previous + // attempt may have POSTed and saved the mapping while failing to save + // the queue row. + check_run_id = shared + .state + .backend + .job_check_run_id(update.run_id, &update.job_id) + .await + .ok() + .flatten(); + if let Some(id) = check_run_id { + persist_check_run_id(shared, owner, update, id).await?; + } + } + if check_run_id.is_none() { + // Crash-after-POST reconciliation: adopt the check run this row + // already created (matched by its `external_id`), never a same-named + // check from another workflow or app. + let external_id = check_run_external_id(update); + check_run_id = + find_existing_check_run(shared, breaker, token, repo, sha, &external_id).await?; + if let Some(id) = check_run_id { + persist_check_run_id(shared, owner, update, id).await?; } - let path = format!("check-runs/{check_run_id}"); - if let Err(error) = send_github_check_request( + } + + if update.payload.get("kind").and_then(Value::as_str) == Some("workflow_failure") { + // Synthetic workflow-evaluation failures have no run row; keep the + // body byte-identical to the pre-queue reporter. + let body = serde_json::json!({ + "name": name, + "head_sha": sha, + "status": "completed", + "conclusion": "failure", + "completed_at": chrono::Utc::now().to_rfc3339(), + "output": { + "title": "Workflow evaluation failed", + "summary": update.payload.get("summary").and_then(Value::as_str).unwrap_or(""), + } + }); + send_check_run_body( shared, - &shared.state.github_breaker, + owner, + breaker, token, repo, - reqwest::Method::PATCH, - &path, - &body, + sha, + name, + update, + check_run_id, + body, ) - .await - { - warn!( - %run_id, - %job_id, + .await?; + finish_check_run_update(shared, owner, update).await?; + return Ok(()); + } + + match status { + // `pending` is a held job (needs/approval); it reports as a queued + // check exactly as intake did. Only terminal statuses conclude one. + "queued" | "pending" | "in_progress" => { + let mut body = if status == "in_progress" { + serde_json::json!({"status":"in_progress","started_at":chrono::Utc::now().to_rfc3339(),"output":{"title":name,"summary":"Running in preloop."}}) + } else { + serde_json::json!({"name":name,"head_sha":sha,"status":"queued","output":{"title":name,"summary":format!("Waiting for a preloop runner.\n\njob_id: `{}`",update.job_id.0)}}) + }; + if let Some(url) = run_details_url(update.run_id) { + body["details_url"] = Value::String(url); + } + send_check_run_body( + shared, + owner, + breaker, + token, + repo, + sha, + name, + update, check_run_id, - %error, - "Failed to requeue GitHub check run" - ); - return Err(error); + body, + ) + .await?; + } + _ => { + let info = shared + .state + .backend + .run_dispatch_info(update.run_id) + .await + .map_err(|e| anyhow::anyhow!("{e:?}"))?; + let conclusion = match status { + "success" => "success", + "failure" => "failure", + "cancelled" => "cancelled", + "skipped" => "skipped", + _ => "failure", + }; + let (title, summary, started_at, completed_at, annotations) = if let Some(info) = info { + let job = info.jobs.iter().find(|job| job.job_id == update.job_id); + let steps = job.map(|j| j.steps.clone()).unwrap_or_default(); + let (annotations, mut global_issues) = + timeline_annotations(shared, update.run_id, &update.job_id).await; + if let Some(detail) = job.and_then(|j| j.detail.as_ref()) { + for annotation in &detail.annotations { + let message = annotation + .get("message") + .and_then(Value::as_str) + .map(str::to_owned) + .unwrap_or_else(|| annotation.to_string()); + global_issues.push(format!("- {}", markdown_cell(&message))); + } + } + let summary = check_summary(conclusion, &steps, &global_issues, &update.job_id); + let title = job + .and_then(|j| j.display_name.as_deref()) + .unwrap_or(name) + .to_owned(); + let started_at = steps + .iter() + .filter_map(|step| step.started_at) + .min() + .or_else(|| info.started_at.map(chrono::DateTime::::from)); + let completed_at = steps + .iter() + .filter_map(|step| step.finished_at) + .max() + .or_else(|| info.completed_at.map(chrono::DateTime::::from)) + .unwrap_or_else(chrono::Utc::now); + (title, summary, started_at, completed_at, annotations) + } else { + ( + name.to_owned(), + update + .payload + .get("summary") + .and_then(Value::as_str) + .unwrap_or("") + .to_owned(), + None, + chrono::Utc::now(), + Vec::new(), + ) + }; + // Annotations travel 50 per request, exactly as before; only the + // final request concludes the check run. + let chunks: Vec<&[Value]> = if annotations.is_empty() { + vec![&[]] + } else { + annotations.chunks(50).collect() + }; + let mut id = check_run_id; + for (index, chunk) in chunks.iter().enumerate() { + let last = index + 1 == chunks.len(); + let mut body = serde_json::json!({ + "output": {"title": title, "summary": summary, "annotations": chunk} + }); + if last { + body["status"] = Value::String("completed".to_owned()); + body["conclusion"] = Value::String(conclusion.to_owned()); + body["completed_at"] = Value::String(completed_at.to_rfc3339()); + if let Some(started_at) = started_at { + body["started_at"] = Value::String(started_at.to_rfc3339()); + } + if let Some(url) = run_details_url(update.run_id) { + body["details_url"] = Value::String(url); + } + } else if id.is_none() { + // Creating a check run requires a status; annotations + // arrive over the follow-up PATCHes. + body["status"] = Value::String("queued".to_owned()); + } + id = send_check_run_body( + shared, owner, breaker, token, repo, sha, &title, update, id, body, + ) + .await?; + } } - } else { - info!( - %run_id, - %job_id, - check_run_id, - "Mock requeued GitHub check run" - ); } + finish_check_run_update(shared, owner, update).await?; Ok(()) } -async fn find_existing_check_run( + +/// The in-memory annotation stream for one job, split into GitHub annotation +/// payloads and free-text failure details (annotations without a file). +async fn timeline_annotations( shared: &Arc, + run_id: RunId, + job_id: &JobId, +) -> (Vec, Vec) { + let inner = shared.state.inner.lock().await; + let mut annotations = Vec::new(); + let mut global_issues = Vec::new(); + if let Some(events) = inner.timeline_events.get(&run_id) { + for event in events { + if let NdjsonEvent::Annotation { + job_id: event_job_id, + level, + message, + file, + line, + .. + } = event + && event_job_id == job_id + { + let level_str = match level { + AnnotationLevel::Notice => "notice", + AnnotationLevel::Warning => "warning", + AnnotationLevel::Error => "failure", + }; + if let Some(file_path) = file { + let line_num = line.unwrap_or(1); + annotations.push(serde_json::json!({ + "path": file_path, + "start_line": line_num, + "end_line": line_num, + "annotation_level": level_str, + "message": message, + })); + } else { + global_issues.push(format!("**{}**: {}", level_str.to_uppercase(), message)); + } + } + } + } + (annotations, global_issues) +} + +/// Send one Checks API request for `update`, creating the check run on POST. +/// Returns the check-run id (persisted alongside the queue row). +#[allow(clippy::too_many_arguments)] +async fn send_check_run_body( + shared: &Arc, + owner: &str, + breaker: &crate::github_breaker::GithubBreaker, token: &str, repo: &str, sha: &str, name: &str, + update: &crate::control::types::CheckRunUpdate, + check_run_id: Option, + mut body: Value, ) -> anyhow::Result> { - let payload = match send_github_check_request( - shared, - &shared.state.github_breaker, - token, - repo, - reqwest::Method::GET, - &format!("commits/{sha}/check-runs"), - &Value::Null, - ) - .await - { - Ok(payload) => payload, - Err(error) if is_check_run_not_found(&error) => return Ok(None), - Err(error) => return Err(error), + let (method, path) = match check_run_id { + Some(id) => (reqwest::Method::PATCH, format!("check-runs/{id}")), + None => { + // Creation requires the check name and commit; `external_id` is + // what reconciliation matches on after a crash. + body["name"] = Value::String(name.to_owned()); + body["head_sha"] = Value::String(sha.to_owned()); + body["external_id"] = Value::String(check_run_external_id(update)); + (reqwest::Method::POST, "check-runs".to_owned()) + } }; - Ok(payload - .get("check_runs") - .and_then(Value::as_array) - .and_then(|check_runs| { - check_runs.iter().find_map(|check_run| { - (check_run.get("name").and_then(Value::as_str) == Some(name) - && check_run.get("head_sha").and_then(Value::as_str) == Some(sha)) - .then(|| check_run.get("id").and_then(Value::as_u64)) - .flatten() - }) - })) + // Hold the row through the request: if it expired and another sender + // re-leased it, that sender owns delivery, and a POST from here would + // create a second check run. + let renewed = shared + .state + .backend + .renew_check_run_update( + owner, + update.run_id, + &update.job_id, + CHECK_RUN_LEASE_RENEWAL, + ) + .await + .map_err(|e| anyhow::anyhow!("{e:?}"))?; + if !renewed { + anyhow::bail!(LOST_CHECK_RUN_LEASE); + } + let response = + match send_github_check_request(shared, breaker, token, repo, method, &path, &body).await { + Ok(response) => response, + Err(error) => { + // An early 404 is usually a just-created check still + // replicating: keep the id and let the backoff retry the + // PATCH. Only a persistent 404 drops it. + if let Some(stale) = check_run_id.filter(|_| { + is_check_run_not_found(&error) && update.attempts >= CHECK_RUN_NOT_FOUND_RETRIES + }) { + let _ = shared + .state + .backend + .clear_check_run_update_id(owner, update.run_id, &update.job_id, stale) + .await; + let _ = shared + .state + .backend + .clear_job_check_run(update.run_id, &update.job_id, stale) + .await; + } + return Err(error); + } + }; + let Some(id) = check_run_id else { + let id = match response.get("id").and_then(Value::as_u64) { + Some(id) => id, + None => { + let external_id = check_run_external_id(update); + find_existing_check_run(shared, breaker, token, repo, sha, &external_id) + .await? + .ok_or_else(|| anyhow::anyhow!("GitHub check-run POST returned no id"))? + } + }; + persist_check_run_id(shared, owner, update, id).await?; + return Ok(Some(id)); + }; + Ok(Some(id)) +} + +/// Record a GitHub check-run id on both the queue row (version-guarded) and +/// the job mapping. +async fn persist_check_run_id( + shared: &Arc, + owner: &str, + update: &crate::control::types::CheckRunUpdate, + id: u64, +) -> anyhow::Result<()> { + shared + .state + .backend + .set_check_run_update_id(owner, update.run_id, &update.job_id, update.version, id) + .await + .map_err(|e| anyhow::anyhow!("{e:?}"))?; + shared + .state + .backend + .set_job_check_run(update.run_id, &update.job_id, id) + .await + .map_err(|e| anyhow::anyhow!("saved check-run id {id} but job mapping failed: {e:?}"))?; + Ok(()) +} + +async fn finish_check_run_update( + shared: &Arc, + owner: &str, + update: &crate::control::types::CheckRunUpdate, +) -> anyhow::Result<()> { + shared + .state + .backend + .finish_check_run_update(owner, update.run_id, &update.job_id, update.version) + .await + .map_err(|e| anyhow::anyhow!("{e:?}")) } /// Report a permanent failure check run to GitHub (e.g. invalid workflow YAML, expression failure). @@ -633,37 +1138,29 @@ pub async fn report_check_run_permanent_failure( name: &str, summary: &str, ) -> anyhow::Result<()> { - let token = resolve_check_run_token(shared, repo).await; - if let Some(token) = &token { - let body = serde_json::json!({ + let run_id = RunId::new(); + let update = crate::control::types::CheckRunUpdateInput { + run_id, + job_id: JobId(format!("__workflow_failure__:{name}")), + installation_id: 0, + check_run_id: None, + version: 0, + payload: serde_json::json!({ + "kind": "workflow_failure", + "repository": repo, + "sha": sha, "name": name, - "head_sha": sha, "status": "completed", - "conclusion": "failure", - "completed_at": chrono::Utc::now().to_rfc3339(), - "output": { - "title": "Workflow evaluation failed", - "summary": summary, - } - }); - let existing = find_existing_check_run(shared, token, repo, sha, name).await?; - let (method, path) = match existing { - Some(check_run_id) => (reqwest::Method::PATCH, format!("check-runs/{check_run_id}")), - None => (reqwest::Method::POST, "check-runs".to_owned()), - }; - send_github_check_request( - shared, - &shared.state.github_breaker, - token, - repo, - method, - &path, - &body, - ) - .await?; - } else { - info!(%repo, %name, "GitHub token not configured, mock failure check run recorded"); - } + "summary": summary, + }), + }; + shared + .state + .backend + .enqueue_check_run_update(update) + .await + .map_err(|e| anyhow::anyhow!("{e:?}"))?; + shared.state.events_dirty.notify_one(); Ok(()) } @@ -673,148 +1170,58 @@ pub async fn report_check_runs_for_run( run_id: RunId, reused_check_run: Option<(JobId, u64)>, ) { - let (repository, sha, jobs) = { - // The rerun reports checks even when every job is an expandable - // placeholder — those mint nothing here, but their materialized legs - // report later and need the flag. - if let Err(error) = shared - .state - .backend - .set_reports_check_runs(run_id, true) - .await - { - warn!(%run_id, ?error, "failed to stamp reports_check_runs for rerun"); - } - let outcome = shared + if let Err(error) = shared + .state + .backend + .set_reports_check_runs(run_id, true) + .await + { + warn!(%run_id, ?error, "failed to stamp reports_check_runs for rerun"); + return; + } + if let Some((job_id, check_run_id)) = reused_check_run + && let Err(error) = shared .state .backend - .run_dispatch_info(run_id) + .set_job_check_run(run_id, &job_id, check_run_id) .await - .map_err(crate::ApiError::from) - .ok() - .flatten(); - match outcome { - Some(info) => ( - info.repository, - info.sha, - // Expandable nodes (deferred matrices, reusable callers) are - // placeholders: expansion replaces them, and their - // materialized legs mint their own checks. A `queued` check - // minted here would strand on GitHub (no delete API). - info.jobs - .into_iter() - .filter(|job| !job.placeholder) - .map(|job| (job.job_id, job.status)) - .collect::>(), - ), - None => return, - } - }; - - for (job_id, status) in jobs { - if let Some((reused_job_id, check_run_id)) = &reused_check_run { - if reused_job_id == &job_id { - if let Err(error) = report_existing_check_run_queued( - shared, - &repository, - &job_id, - run_id, - *check_run_id, - ) + { + warn!(%run_id, %job_id, ?error, "failed to persist reused check-run id"); + } + if let Ok(Some(info)) = shared.state.backend.run_dispatch_info(run_id).await { + for job in info.jobs.into_iter().filter(|job| !job.placeholder) { + if let Err(error) = shared + .state + .backend + .append_check_run_projection(run_id, Some(&job.job_id)) .await - { - warn!(%run_id, %job_id, ?error, "failed to requeue GitHub check run"); - } - } else if let Err(error) = - report_check_run_queued(shared, &repository, &sha, &job_id, run_id).await { - warn!(%run_id, %job_id, ?error, "failed to report queued GitHub check run"); + warn!(%run_id, job_id=%job.job_id, ?error, "failed to append rerun check projection wake"); } - } else if let Err(error) = - report_check_run_queued(shared, &repository, &sha, &job_id, run_id).await - { - warn!(%run_id, %job_id, ?error, "failed to report queued GitHub check run"); - } - - if status.is_terminal() { - report_check_run_completed(shared, run_id, &job_id, status).await; } + shared.state.events_dirty.notify_one(); } } /// Report check run status to in_progress on GitHub or simulate it locally. +/// The acquire transition writes `job.started.v1` in its own transaction; the +/// durable projector observes it and the sender performs the PATCH +/// asynchronously. This wake covers a projector that consumed that event +/// before the run's `reports_check_runs` flag landed. pub async fn report_check_run_in_progress( shared: &Arc, run_id: RunId, job_id: &JobId, ) { - let (repo, check_run_id, job_name) = { - let backend = &shared.state.backend; - let Some(check_run_id) = backend - .job_check_run_id(run_id, job_id) - .await - .ok() - .flatten() - else { - return; - }; - let repo = backend - .submission_fields(run_id) - .await - .ok() - .flatten() - .map(|fields| fields.repository); - let Some(repo) = repo else { - return; - }; - let job_name = backend - .job_display_name(run_id, job_id) - .await - .ok() - .flatten() - .unwrap_or_else(|| job_id.0.clone()); - (repo, check_run_id, job_name) - }; - - let token = resolve_check_run_token(shared, &repo).await; - if let Some(token) = &token { - let details_url = run_details_url(run_id); - - let started_at = chrono::Utc::now().to_rfc3339(); - let mut body = serde_json::json!({ - "status": "in_progress", - "started_at": started_at, - "output": { - "title": job_name, - "summary": "Running in preloop." - } - }); - if let Some(url) = details_url { - body["details_url"] = serde_json::json!(url); - } - - let path = format!("check-runs/{}", check_run_id); - if let Err(e) = send_github_check_request( - shared, - &shared.state.github_lifecycle_breaker, - token, - &repo, - reqwest::Method::PATCH, - &path, - &body, - ) + if let Err(error) = shared + .state + .backend + .append_check_run_projection(run_id, Some(job_id)) .await - { - warn!( - %run_id, - %job_id, - check_run_id, - error = %e, - "Failed to update GitHub check run to in_progress" - ); - } + { + warn!(%run_id, %job_id, ?error, "failed to append in-progress check projection wake"); } else { - info!(%run_id, %job_id, check_run_id, "Mock updated check run to in_progress"); + shared.state.events_dirty.notify_one(); } } @@ -875,207 +1282,26 @@ fn check_summary( summary } /// Report check run status to completed on GitHub or simulate it locally. +/// +/// The terminal transition already wrote a durable event; this wake covers a +/// projector that consumed it before the run's `reports_check_runs` flag +/// landed (a job concluded during submit) — the sender renders the row's +/// final state. pub async fn report_check_run_completed( shared: &Arc, run_id: RunId, job_id: &JobId, - status: ExecutionStatus, + _status: ExecutionStatus, ) { - // Backend: one indexed dispatch read. Node-local: `timeline_events` for - // annotations. Read each under its own owner. - let (repo, _check_run_id, job_name, steps, started_at, completed_at, detail) = { - let outcome = shared - .state - .backend - .run_dispatch_info(run_id) - .await - .map_err(crate::ApiError::from) - .ok() - .flatten(); - let Some(info) = outcome else { - return; - }; - // A job minted on demand (deferred-matrix leg, reusable callee) may - // have no dispatch row at all — proceed with run-level defaults so - // `ensure_check_run_mapped` below can mint for it. - let job = info.jobs.iter().find(|job| job.job_id == *job_id); - let check_run_id = job.and_then(|job| job.check_run_id); - // `project_run`'s per-job projection, minus the pieces it derived - // from a second pass over the whole run: name, status conclusion and - // the latest attempt's step manifest. - let mut detail = - job.and_then(|job| job.detail.clone()) - .unwrap_or(crate::models::JobDetail { - job_id: job_id.0.clone(), - name: job - .and_then(|job| job.display_name.clone()) - .unwrap_or_else(|| job_id.0.clone()), - conclusion: crate::runtime_scheduling::status_string( - job.map(|job| job.status) - .unwrap_or(ExecutionStatus::Pending), - ), - steps: Vec::new(), - annotations: Vec::new(), - }); - detail.job_id = job_id.0.clone(); - detail.name = job - .and_then(|job| job.display_name.clone()) - .unwrap_or_else(|| job_id.0.clone()); - detail.conclusion = crate::runtime_scheduling::status_string( - job.map(|job| job.status) - .unwrap_or(ExecutionStatus::Pending), - ); - let job_steps: Vec = - job.map(|job| job.steps.clone()).unwrap_or_default(); - if !job_steps.is_empty() { - detail.steps = job_steps.clone(); - } - let job_name = detail.name.clone(); - let started_at = job_steps - .iter() - .filter_map(|step| step.started_at) - .min() - .or_else(|| info.started_at.map(chrono::DateTime::from)); - let completed_at = job_steps - .iter() - .filter_map(|step| step.finished_at) - .max() - .or_else(|| info.completed_at.map(chrono::DateTime::from)) - .unwrap_or_else(chrono::Utc::now); - ( - info.repository, - check_run_id, - job_name, - job_steps, - started_at, - completed_at, - Some(detail), - ) - }; - - let (annotations, global_issues) = { - let inner = shared.state.inner.lock().await; - let mut annotations = Vec::new(); - let mut global_issues = Vec::new(); - if let Some(events) = inner.timeline_events.get(&run_id) { - for event in events { - if let NdjsonEvent::Annotation { - job_id: event_job_id, - level, - message, - file, - line, - .. - } = event - && event_job_id == job_id - { - let level_str = match level { - AnnotationLevel::Notice => "notice", - AnnotationLevel::Warning => "warning", - AnnotationLevel::Error => "failure", - }; - if let Some(file_path) = file { - let line_num = line.unwrap_or(1); - annotations.push(serde_json::json!({ - "path": file_path, - "start_line": line_num, - "end_line": line_num, - "annotation_level": level_str, - "message": message, - })); - } else { - global_issues.push(format!( - "**{}**: {}", - level_str.to_uppercase(), - message - )); - } - } - } - } - if let Some(detail) = &detail { - for annotation in &detail.annotations { - let message = annotation - .get("message") - .and_then(Value::as_str) - .map(str::to_owned) - .unwrap_or_else(|| annotation.to_string()); - global_issues.push(format!("- {}", markdown_cell(&message))); - } - } - (annotations, global_issues) - }; - - // Jobs materialized after intake (runtime matrix fan-out, reusable callee - // jobs) have no check run yet; mint one instead of dropping the report. - // A cancelled expandable placeholder mints a `cancelled` check — GitHub - // has no delete API, so concluding it is better than leaving it queued. - let Some(check_run_id) = ensure_check_run_mapped(shared, run_id, job_id).await else { - return; - }; - - let conclusion = match status { - ExecutionStatus::Success => "success", - ExecutionStatus::Failure => "failure", - ExecutionStatus::Cancelled => "cancelled", - ExecutionStatus::Skipped => "skipped", - _ => "failure", - }; - let summary = check_summary(conclusion, &steps, &global_issues, job_id); - - let token = resolve_check_run_token(shared, &repo).await; - if let Some(token) = &token { - let path = format!("check-runs/{check_run_id}"); - let chunks: Vec<&[Value]> = if annotations.is_empty() { - vec![&[]] - } else { - annotations.chunks(50).collect() - }; - for (index, chunk) in chunks.iter().enumerate() { - let last = index + 1 == chunks.len(); - let mut body = serde_json::json!({ - "output": { - "title": job_name, - "summary": summary, - "annotations": chunk, - } - }); - if last { - body["status"] = serde_json::json!("completed"); - body["conclusion"] = serde_json::json!(conclusion); - body["completed_at"] = serde_json::json!(completed_at.to_rfc3339()); - if let Some(started_at) = started_at { - body["started_at"] = serde_json::json!(started_at.to_rfc3339()); - } - if let Some(url) = run_details_url(run_id) { - body["details_url"] = serde_json::json!(url); - } - } - if let Err(error) = - send_github_check_completion(shared, token, &repo, &path, &body).await - { - warn!( - %run_id, - %job_id, - check_run_id, - annotation_batch = index + 1, - annotation_batches = chunks.len(), - %error, - "Failed to update GitHub check run" - ); - return; - } - } + if let Err(error) = shared + .state + .backend + .append_check_run_projection(run_id, Some(job_id)) + .await + { + warn!(%run_id, %job_id, ?error, "failed to append completed check projection wake"); } else { - info!( - %run_id, - %job_id, - check_run_id, - conclusion, - annotations_count = annotations.len(), - global_issues_count = global_issues.len(), - "Mock updated check run to completed" - ); + shared.state.events_dirty.notify_one(); } } @@ -3873,6 +4099,9 @@ jobs: #[tokio::test] async fn malformed_pull_request_workflow_reports_head_sha() { + // See `malformed_pull_request_workflow_failure_is_deduplicated`: the + // PR-files lookup must not hit another test's GitHub stub. + let _env = crate::state::GITHUB_ENV_LOCK.lock().await; let temp = tempfile::tempdir().unwrap(); let ws_dir = temp.path().join("ws"); std::fs::create_dir_all(ws_dir.join(".github/workflows")).unwrap(); @@ -3929,6 +4158,10 @@ jobs: #[tokio::test] async fn malformed_pull_request_workflow_failure_is_deduplicated() { + // `PRELOOP_GITHUB_API_URL` is process-global; this scenario expects no + // stub to answer the PR-files lookup, so it must not race a test that + // installs one. + let _env = crate::state::GITHUB_ENV_LOCK.lock().await; let temp = tempfile::tempdir().unwrap(); let ws_dir = temp.path().join("ws"); std::fs::create_dir_all(ws_dir.join(".github/workflows")).unwrap(); @@ -4667,20 +4900,40 @@ jobs: #[tokio::test] async fn materialized_job_mints_check_run_when_intake_reported() { let (_temp, shared, run_id) = mint_fixture(true, None).await; + let job = JobId("build".to_owned()); - // A leg materialized by a deferred matrix has a check run minted - // on demand — no GitHub token resolves in tests, so the mock id is - // fine; the assertion is that the mapping exists at all. - let leg = JobId("build (linux)".to_owned()); - let id = ensure_check_run_mapped(&shared, run_id, &leg).await; - assert!(id.is_some(), "reported runs mint checks for late jobs"); + // Handlers never mint inline any more: there is no GitHub call and no + // persisted id before the sender runs. + assert_eq!( + ensure_check_run_mapped(&shared, run_id, &job).await, + None, + "a request handler must not mint a check run" + ); + + // The durable queued report stamps the run and appends a projection + // wake; the projector materializes the desired row, and the sender + // (exercised by the sender tests) creates the GitHub check from it. + report_check_run_queued( + &shared, + "owner/repo", + "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2", + &job, + run_id, + ) + .await + .unwrap(); + let row = wait_for_check_run_row(&shared, run_id, &job).await; + assert_eq!(row.payload["status"], "queued"); + assert_eq!(row.payload["name"], "build"); + assert_eq!(row.payload["repository"], "owner/repo"); + assert_eq!( + row.payload["sha"], "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2", + "the projected row carries the run's check sha" + ); let runs = shared.state.test_tx().await.runs; assert!( - runs.get(&run_id) - .unwrap() - .job_check_run_ids - .contains_key(&leg), - "the minted id must be recorded for lifecycle PATCHes" + runs.get(&run_id).unwrap().reports_check_runs, + "the queued report must stamp the run so later jobs report too" ); } @@ -4701,20 +4954,602 @@ jobs: #[tokio::test] async fn terminal_report_mints_missing_check_run_for_late_job() { let (_temp, shared, run_id) = mint_fixture(true, None).await; - let leg = JobId("build (linux)".to_owned()); + let job = JobId("build".to_owned()); + + // A matrix leg that skips at promotion never dispatches; the only + // report it ever gets is the completion one. It must reach the queue + // even though the job settled before the reporting flag landed (a + // `JobStatus` event would be stamped stale; the projection wake is + // not). + shared + .state + .test_db_mutate(|db| { + db.execute( + "UPDATE jobs SET status = 'skipped' WHERE run_id = ?1 AND job_id = ?2", + rusqlite::params![run_id.to_string(), "build"], + ) + }) + .await + .unwrap(); + report_check_run_completed(&shared, run_id, &job, ExecutionStatus::Skipped).await; - // A matrix leg that skips at promotion never dispatches, so the only - // report it ever gets is the completion one — which must mint the - // check rather than drop the report. - report_check_run_completed(&shared, run_id, &leg, ExecutionStatus::Skipped).await; + let row = wait_for_check_run_row(&shared, run_id, &job).await; + assert_eq!( + row.payload["status"], "skipped", + "a terminal report must queue its final state for the sender" + ); + } - let runs = shared.state.test_tx().await.runs; + /// Wait for the durable projector (which owns the consumer lease in the + /// background) to materialize a queue row for `run_id`/`job_id`. + async fn wait_for_check_run_row( + shared: &std::sync::Arc, + run_id: RunId, + job_id: &JobId, + ) -> crate::control::types::CheckRunUpdate { + for _ in 0..120 { + if let Ok(rows) = shared + .state + .backend + .lease_check_run_updates("test-probe", std::time::Duration::from_millis(1), 100) + .await + && let Some(row) = rows + .into_iter() + .find(|row| row.run_id == run_id && row.job_id == *job_id) + { + return row; + } + tokio::time::sleep(std::time::Duration::from_millis(25)).await; + } + panic!("check-run row for {run_id}/{job_id} was never projected"); + } + + // ── Sender acceptance tests ──────────────────────────────────────── + + type StubRequests = std::sync::Arc>>; + + /// Fake GitHub API: records every request and answers from + /// `handler(method, path, body)`. Returns the base URL to install as + /// `PRELOOP_GITHUB_API_URL`. + async fn start_github_stub( + handler: impl Fn(&str, &str, &Value) -> (u16, Vec<(&'static str, String)>, Value) + + Send + + Sync + + 'static, + ) -> (String, StubRequests) { + let requests: StubRequests = std::sync::Arc::new(parking_lot::Mutex::new(Vec::new())); + let recorded = requests.clone(); + let handler = std::sync::Arc::new(handler); + let app = axum::Router::new().fallback(move |request: axum::extract::Request| { + let recorded = recorded.clone(); + let handler = handler.clone(); + async move { + let method = request.method().to_string(); + let path = request.uri().path().to_owned(); + let bytes = axum::body::to_bytes(request.into_body(), 1 << 20) + .await + .unwrap_or_default(); + let body: Value = serde_json::from_slice(&bytes).unwrap_or(Value::Null); + recorded + .lock() + .push((method.clone(), path.clone(), body.clone())); + let (status, headers, response_body) = handler(&method, &path, &body); + let mut response = axum::response::Response::new(axum::body::Body::from( + response_body.to_string(), + )); + *response.status_mut() = axum::http::StatusCode::from_u16(status).unwrap(); + for (name, value) in headers { + response.headers_mut().insert( + axum::http::HeaderName::from_bytes(name.as_bytes()).unwrap(), + axum::http::HeaderValue::from_str(&value).unwrap(), + ); + } + response + } + }); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { + axum::serve(listener, app).await.unwrap(); + }); + (format!("http://{addr}"), requests) + } + + fn check_run_writes(requests: &StubRequests) -> Vec<(String, String, Value)> { + requests + .lock() + .iter() + .filter(|(method, _, _)| method == "POST" || method == "PATCH") + .cloned() + .collect() + } + + async fn seed_check_run_update( + shared: &std::sync::Arc, + run_id: RunId, + job_id: &JobId, + version: i64, + status: &str, + check_run_id: Option, + ) { + shared + .state + .backend + .enqueue_check_run_update(crate::control::types::CheckRunUpdateInput { + run_id, + job_id: job_id.clone(), + installation_id: 0, + check_run_id, + version, + payload: serde_json::json!({ + "repository": "owner/repo", + "sha": "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2", + "job_id": job_id.0, + "status": status, + "name": "build", + }), + }) + .await + .unwrap(); + } + + async fn drain_sender(shared: &std::sync::Arc) -> usize { + CheckRunSender::new().drain_once(shared).await + } + + /// (b) queued → in_progress → completed committed before the sender runs + /// coalesces to ONE Checks API write carrying the final state. + #[tokio::test] + async fn sender_coalesces_versions_into_one_final_request() { + let _env = crate::state::GITHUB_ENV_LOCK.lock().await; + let (api, requests) = start_github_stub(|method, _path, _body| match method { + "GET" => (200, vec![], serde_json::json!({"check_runs": []})), + _ => (201, vec![], serde_json::json!({"id": 4242})), + }) + .await; + let _api = crate::state::TestEnvVar::set("PRELOOP_GITHUB_API_URL", &api); + let _token = crate::state::TestEnvVar::set("PRELOOP_GITHUB_TOKEN", "test-token"); + let (_temp, shared, run_id) = mint_fixture(true, None).await; + let job = JobId("build".to_owned()); + for (version, status) in [(1, "queued"), (2, "in_progress"), (3, "success")] { + seed_check_run_update(&shared, run_id, &job, version, status, None).await; + } + + assert_eq!(drain_sender(&shared).await, 1); + let writes = check_run_writes(&requests); + assert_eq!(writes.len(), 1, "coalesced to one write: {writes:?}"); + assert_eq!(writes[0].0, "POST"); + assert_eq!(writes[0].2["status"], "completed"); + assert_eq!(writes[0].2["conclusion"], "success"); + assert!( + shared + .state + .backend + .lease_check_run_updates("probe", std::time::Duration::from_millis(1), 10) + .await + .unwrap() + .is_empty(), + "the sent row is deleted" + ); + } + + /// (d) a 429 with Retry-After parks the row for the advertised delay and + /// the next attempt succeeds. + #[tokio::test] + async fn sender_retries_after_retry_after_then_succeeds() { + let _env = crate::state::GITHUB_ENV_LOCK.lock().await; + let attempts = std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let attempts_for_stub = attempts.clone(); + let (api, requests) = start_github_stub(move |method, _path, _body| { + if method == "GET" { + return (200, vec![], serde_json::json!({"check_runs": []})); + } + if attempts_for_stub.fetch_add(1, std::sync::atomic::Ordering::SeqCst) == 0 { + ( + 429, + vec![("retry-after", "1".to_owned())], + serde_json::json!({"message": "You have exceeded a secondary rate limit"}), + ) + } else { + (201, vec![], serde_json::json!({"id": 777})) + } + }) + .await; + let _api = crate::state::TestEnvVar::set("PRELOOP_GITHUB_API_URL", &api); + let _token = crate::state::TestEnvVar::set("PRELOOP_GITHUB_TOKEN", "test-token"); + let (_temp, shared, run_id) = mint_fixture(true, None).await; + let job = JobId("build".to_owned()); + seed_check_run_update(&shared, run_id, &job, 1, "queued", None).await; + + assert_eq!(drain_sender(&shared).await, 1); + assert_eq!( + check_run_writes(&requests).len(), + 1, + "first attempt is throttled" + ); + assert_eq!( + drain_sender(&shared).await, + 0, + "the row is not due until Retry-After elapses" + ); + assert_eq!(check_run_writes(&requests).len(), 1); + tokio::time::sleep(std::time::Duration::from_millis(1_150)).await; + assert_eq!(drain_sender(&shared).await, 1); + assert_eq!( + check_run_writes(&requests).len(), + 2, + "retried and succeeded" + ); + assert!( + shared + .state + .backend + .lease_check_run_updates("probe", std::time::Duration::from_millis(1), 10) + .await + .unwrap() + .is_empty(), + "the successful retry deletes the row" + ); + } + + /// (e) crash after POST before the id was saved: the next attempt finds + /// the existing check run (or the persisted job mapping) and PATCHes it; + /// it never POSTs a duplicate. + #[tokio::test] + async fn sender_reconciles_instead_of_second_post() { + let _env = crate::state::GITHUB_ENV_LOCK.lock().await; + // The row's own check is listed after a same-named check another + // workflow created on the same commit; only the `external_id` match + // may be adopted. The id is filled in once the fixture exists. + let own_external_id = std::sync::Arc::new(parking_lot::Mutex::new(String::new())); + let stub_external_id = own_external_id.clone(); + let (api, requests) = start_github_stub(move |method, path, _body| { + if method == "GET" { + let existing = if path.contains("commits/") { + serde_json::json!({"total_count": 2, "check_runs": [ + { + "id": 6001, + "name": "build", + "external_id": "another-workflow-run:build", + "head_sha": "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2", + }, + { + "id": 7001, + "name": "build", + "external_id": *stub_external_id.lock(), + "head_sha": "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2", + }, + ]}) + } else { + serde_json::json!({}) + }; + return (200, vec![], existing); + } + (200, vec![], serde_json::json!({"id": 7001})) + }) + .await; + let _api = crate::state::TestEnvVar::set("PRELOOP_GITHUB_API_URL", &api); + let _token = crate::state::TestEnvVar::set("PRELOOP_GITHUB_TOKEN", "test-token"); + let (_temp, shared, run_id) = mint_fixture(true, None).await; + let job = JobId("build".to_owned()); + *own_external_id.lock() = format!("{run_id}:{}", job.0); + + // The check run already exists on GitHub but the row has no id. + seed_check_run_update(&shared, run_id, &job, 1, "queued", None).await; + assert_eq!(drain_sender(&shared).await, 1); + let writes = check_run_writes(&requests); + assert_eq!(writes.len(), 1, "one write only: {writes:?}"); + assert_eq!(writes[0].0, "PATCH"); + assert!(writes[0].1.ends_with("/check-runs/7001")); + assert_eq!( + shared + .state + .backend + .job_check_run_id(run_id, &job) + .await + .unwrap(), + Some(7001), + "the reconciled id is persisted" + ); + + // A persisted job mapping alone (no GitHub lookup) also PATCHes. + let (_temp2, shared2, run_id2) = mint_fixture(true, None).await; + shared2 + .state + .backend + .set_job_check_run(run_id2, &job, 7002) + .await + .unwrap(); + seed_check_run_update(&shared2, run_id2, &job, 1, "queued", None).await; + let before = requests.lock().len(); + assert_eq!(drain_sender(&shared2).await, 1); + let writes: Vec<_> = requests + .lock() + .iter() + .skip(before) + .filter(|(method, _, _)| method == "POST" || method == "PATCH") + .cloned() + .collect(); + assert_eq!(writes.len(), 1); + assert_eq!(writes[0].0, "PATCH"); + assert!(writes[0].1.ends_with("/check-runs/7002")); + assert!( + !requests + .lock() + .iter() + .skip(before) + .any(|(method, _, _)| method == "POST"), + "no duplicate POST" + ); + } + + /// (h) a throttled installation parks only its own rows: the other + /// installation's check run succeeds in the same batch. + #[tokio::test] + async fn throttled_installation_does_not_block_another() { + let _env = crate::state::GITHUB_ENV_LOCK.lock().await; + let (api, requests) = start_github_stub(|method, path, _body| { + if path.contains("/access_tokens") { + let token = if path.contains("/101/") { + "ghs-a" + } else { + "ghs-b" + }; + return ( + 201, + vec![], + serde_json::json!({"token": token, "expires_at": "2099-01-01T00:00:00Z"}), + ); + } + if method == "GET" && path.starts_with("/app/installations/") { + let login = if path.ends_with("/101") { + "owner-a" + } else { + "owner-b" + }; + return ( + 200, + vec![], + serde_json::json!({"account": {"login": login}}), + ); + } + if method == "GET" { + return (200, vec![], serde_json::json!({"check_runs": []})); + } + if path.starts_with("/repos/owner-a/") { + let reset = (std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_secs() + + 60) + .to_string(); + return ( + 403, + vec![ + ("x-ratelimit-remaining", "0".to_owned()), + ("x-ratelimit-reset", reset), + ], + serde_json::json!({"message": "API rate limit exceeded"}), + ); + } + (201, vec![], serde_json::json!({"id": 900})) + }) + .await; + let _api = crate::state::TestEnvVar::set("PRELOOP_GITHUB_API_URL", &api); + let _token = crate::state::TestEnvVar::set("PRELOOP_GITHUB_TOKEN", "test-token"); + + let key = rsa::RsaPrivateKey::new(&mut rand::thread_rng(), 2048).unwrap(); + let mut app_a = crate::github_app::GitHubAppCredentials::for_tests( + "424", + key.clone(), + crate::github_app::MintFailurePolicy::LocalJwt, + ); + app_a.installation_id = Some(101); + let mut app_b = crate::github_app::GitHubAppCredentials::for_tests( + "425", + key, + crate::github_app::MintFailurePolicy::LocalJwt, + ); + app_b.installation_id = Some(202); + let temp = tempfile::tempdir().unwrap(); + let mut state = AppState::new(temp.path().to_path_buf()).await.unwrap(); + state.github_apps = Some(crate::github_app::GitHubApps { + apps: vec![app_a.clone(), app_b], + default_index: 0, + }); + state.github_app = Some(app_a); + let shared = state.shared(); + let job = JobId("build".to_owned()); + let mut run_ids = Vec::new(); + for repo in ["owner-a/repo", "owner-b/repo"] { + let submission = WorkflowSubmission { + workflow_yaml: "on: push\njobs:\n build:\n runs-on: ubuntu-latest\n steps:\n - run: echo hello\n".to_owned(), + event: "push".to_owned(), + repository: repo.to_owned(), + workflow_path: Some(".github/workflows/ci.yml".to_owned()), + sha: "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2".to_owned(), + resolved_sha: Some("a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2".to_owned()), + ..Default::default() + }; + let accepted = crate::submit_run_inner(&shared, submission).await.unwrap(); + shared + .state + .backend + .set_reports_check_runs(accepted.run_id, true) + .await + .unwrap(); + let payload = serde_json::json!({ + "repository": repo, + "sha": "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2", + "job_id": "build", + "status": "queued", + "name": "build", + }); + shared + .state + .backend + .enqueue_check_run_update(crate::control::types::CheckRunUpdateInput { + run_id: accepted.run_id, + job_id: job.clone(), + installation_id: 0, + check_run_id: None, + version: 1, + payload, + }) + .await + .unwrap(); + run_ids.push(accepted.run_id); + } + + assert_eq!(drain_sender(&shared).await, 2, "both rows are leased"); + let writes: Vec<_> = check_run_writes(&requests) + .into_iter() + .filter(|(_, path, _)| path.contains("/check-runs")) + .collect(); + assert_eq!( + writes.len(), + 2, + "one write per installation: all={:?}", + check_run_writes(&requests) + ); + let a_writes = writes + .iter() + .filter(|(_, path, _)| path.starts_with("/repos/owner-a/")) + .count(); + let b_writes = writes + .iter() + .filter(|(_, path, _)| path.starts_with("/repos/owner-b/")) + .count(); + assert_eq!((a_writes, b_writes), (1, 1)); + let writes_before = writes.len(); + assert_eq!( + drain_sender(&shared).await, + 0, + "the throttled installation's row is deferred, not retried immediately" + ); + assert_eq!( + check_run_writes(&requests) + .iter() + .filter(|(_, path, _)| path.contains("/check-runs")) + .count(), + writes_before + ); + // B's row is gone; A's is parked until the reset (not due). + assert!( + shared + .state + .backend + .lease_check_run_updates("probe", std::time::Duration::from_millis(1), 10) + .await + .unwrap() + .is_empty(), + "the successful installation's row is deleted and the throttled one is not due" + ); + } + + /// (j) a run that vanished before the sender ran still delivers its final + /// state from the payload; permanent errors drop the row. + #[tokio::test] + async fn sender_delivers_vanished_run_and_drops_permanent_failures() { + let _env = crate::state::GITHUB_ENV_LOCK.lock().await; + let statuses = std::sync::Arc::new(parking_lot::Mutex::new(Vec::::new())); + let statuses_for_stub = statuses.clone(); + let (api, requests) = start_github_stub(move |method, _path, _body| { + if method == "GET" { + return (200, vec![], serde_json::json!({"check_runs": []})); + } + let status = statuses_for_stub.lock().pop().unwrap_or(201); + (status, vec![], serde_json::json!({"id": 555})) + }) + .await; + let _api = crate::state::TestEnvVar::set("PRELOOP_GITHUB_API_URL", &api); + let _token = crate::state::TestEnvVar::set("PRELOOP_GITHUB_TOKEN", "test-token"); + let temp = tempfile::tempdir().unwrap(); + let state = AppState::new(temp.path().to_path_buf()).await.unwrap(); + let shared = state.shared(); + let job = JobId("build".to_owned()); + + // No `runs` row at all: an archived/deleted run, or the synthetic + // workflow-failure shape. + seed_check_run_update(&shared, RunId::new(), &job, 1, "success", None).await; + assert_eq!(drain_sender(&shared).await, 1); + let writes = check_run_writes(&requests); + assert_eq!(writes.len(), 1); + assert_eq!(writes[0].2["status"], "completed"); + assert_eq!(writes[0].2["conclusion"], "success"); + + // A permanent 422 drops the row after one attempt. + statuses.lock().push(422); + seed_check_run_update(&shared, RunId::new(), &job, 1, "queued", None).await; + assert_eq!(drain_sender(&shared).await, 1); assert!( - runs.get(&run_id) + shared + .state + .backend + .lease_check_run_updates("probe", std::time::Duration::from_millis(1), 10) + .await .unwrap() - .job_check_run_ids - .contains_key(&leg), - "a terminal report must mint the check it reports to" + .is_empty(), + "a 422 must drop the row instead of retrying forever" + ); + } + + /// (a) a webhook push fanning out to many jobs returns without any inline + /// GitHub Checks API call; the durable queue carries the reports instead. + #[tokio::test] + async fn webhook_fanout_makes_zero_inline_check_calls() { + let _env = crate::state::GITHUB_ENV_LOCK.lock().await; + let (api, requests) = + start_github_stub(|_method, _path, _body| (200, vec![], serde_json::json!({}))).await; + let _api = crate::state::TestEnvVar::set("PRELOOP_GITHUB_API_URL", &api); + let _token = crate::state::TestEnvVar::set("PRELOOP_GITHUB_TOKEN", "test-token"); + + let temp = tempfile::tempdir().unwrap(); + let ws_dir = temp.path().join("ws"); + std::fs::create_dir_all(ws_dir.join(".github/workflows")).unwrap(); + let mut workflow = "on: push\njobs:\n".to_owned(); + for index in 0..8 { + workflow.push_str(&format!( + " job{index}:\n runs-on: ubuntu-latest\n steps:\n - run: echo {index}\n" + )); + } + std::fs::write(ws_dir.join(".github/workflows/build.yml"), workflow).unwrap(); + let fixture = WebhookFixture::with_workspace(&temp, ws_dir).await; + + assert_eq!( + fixture.post("delivery-check-fanout", Some("push")).await, + StatusCode::ACCEPTED + ); + assert!( + requests + .lock() + .iter() + .all(|(_, path, _)| !path.contains("check-runs")), + "the webhook handler must not call the Checks API inline: {:?}", + requests.lock() + ); + fixture.drain().await; + let record = fixture + .state + .backend + .get_webhook_delivery("delivery-check-fanout") + .await + .unwrap() + .expect("delivery row"); + assert_eq!(record.state, WebhookDeliveryStatus::Done); + let inner = fixture.state.test_tx().await; + assert_eq!(inner.runs.len(), 1); + assert_eq!( + inner.runs.values().next().unwrap().jobs.len(), + 8, + "all eight jobs fan out" + ); + assert!( + requests + .lock() + .iter() + .all(|(_, path, _)| !path.contains("check-runs")), + "delivery must not call the Checks API inline either" ); } @@ -4722,27 +5557,43 @@ jobs: async fn terminal_check_retries_not_found_after_creation() { let _env = crate::state::GITHUB_ENV_LOCK.lock().await; let attempts = std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0)); - let mock_app = axum::Router::new().route( - "/repos/owner/repo/check-runs/:id", - axum::routing::patch({ - let attempts = attempts.clone(); - move || { + let creates = std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let mock_app = axum::Router::new() + .route( + "/repos/owner/repo/check-runs/:id", + axum::routing::patch({ let attempts = attempts.clone(); - async move { - let attempt = attempts.fetch_add(1, std::sync::atomic::Ordering::SeqCst); - if attempt == 0 { - ( - StatusCode::NOT_FOUND, - Json(serde_json::json!({"message": "Not Found"})), - ) - .into_response() - } else { - Json(serde_json::json!({"id": 7})).into_response() + move || { + let attempts = attempts.clone(); + async move { + let attempt = + attempts.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + if attempt == 0 { + ( + StatusCode::NOT_FOUND, + Json(serde_json::json!({"message": "Not Found"})), + ) + .into_response() + } else { + Json(serde_json::json!({"id": 7})).into_response() + } } } - } - }), - ); + }), + ) + .route( + "/repos/owner/repo/check-runs", + axum::routing::post({ + let creates = creates.clone(); + move || { + let creates = creates.clone(); + async move { + creates.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + Json(serde_json::json!({"id": 99})) + } + } + }), + ); let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); let port = listener.local_addr().unwrap().port(); let server = tokio::spawn(async move { @@ -4769,15 +5620,72 @@ jobs: let _token = crate::state::TestEnvVar::set("PRELOOP_GITHUB_TOKEN", "check-retry-token"); report_check_run_completed(&shared, run_id, &job_id, ExecutionStatus::Skipped).await; + // The projector queues the update in the background; drain the + // sender until the PATCH is retried past its first 404. + tokio::time::timeout(std::time::Duration::from_secs(10), async { + while attempts.load(std::sync::atomic::Ordering::SeqCst) < 2 { + drain_sender(&shared).await; + tokio::time::sleep(std::time::Duration::from_millis(25)).await; + } + }) + .await + .expect("the PATCH is retried after an immediate 404"); assert_eq!( attempts.load(std::sync::atomic::Ordering::SeqCst), 2, "an immediate 404 must be retried instead of stranding the check" ); + assert_eq!( + creates.load(std::sync::atomic::Ordering::SeqCst), + 0, + "an immediate 404 must not abandon the check and POST a duplicate" + ); + assert_eq!( + shared + .state + .backend + .job_check_run_id(run_id, &job_id) + .await + .unwrap(), + Some(7), + "the check-run mapping survives the replication-race 404" + ); server.abort(); } + /// GitHub sends `x-ratelimit-reset` on every response, so a failure with + /// budget left keeps the short exponential backoff; only a spent budget + /// waits for the reset. + #[test] + fn check_retry_delay_waits_for_reset_only_when_budget_is_spent() { + let reset = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_secs() + + 1_800; + let failure = |status: u16, remaining: &str| { + anyhow::anyhow!( + "GitHub Check API failed with status {status} Internal Server Error: {{}}; \ + retry_after=; rate_remaining={remaining}; rate_reset={reset}" + ) + }; + + for (status, remaining) in [(500, "4321"), (404, "4999"), (422, "12"), (502, "")] { + let delay = check_retry_delay(&failure(status, remaining), 0); + assert!( + delay < std::time::Duration::from_secs(1), + "status {status} with remaining={remaining:?} must back off briefly, got {delay:?}" + ); + } + + let delay = check_retry_delay(&failure(403, "0"), 0); + assert!( + delay > std::time::Duration::from_secs(1_700), + "a spent budget waits for the advertised reset, got {delay:?}" + ); + } + #[test] fn report_coords_prefer_status_check_sha() { // Late mints must land on the same commit as intake checks: for PR diff --git a/crates/preloop-runner-server/src/github_app.rs b/crates/preloop-runner-server/src/github_app.rs index 25030406e..b03f364ca 100644 --- a/crates/preloop-runner-server/src/github_app.rs +++ b/crates/preloop-runner-server/src/github_app.rs @@ -731,6 +731,21 @@ pub async fn select_app_for_repo( .next() } +/// The installation id `app` mints tokens for on `repository`'s owner. +/// +/// Exposed so the check-run sender can key per-installation rate budgets on +/// the *real* installation rather than whatever id a queue row happened to +/// carry: `0` is never a valid GitHub installation, and discovery is cached +/// per App+owner, so steady state costs no extra API call. +pub async fn installation_for_repo( + app: &GitHubAppCredentials, + repository: &str, +) -> anyhow::Result { + let (owner, _) = split_repository(repository)?; + let app_jwt = sign_app_jwt(&app.app_id, &app.private_key)?; + installation_id_for(&api_base(), app, &app_jwt, owner).await +} + /// Mint an installation token for `repository`, scoped to `permissions`. /// /// `repository` is an `owner/repo` slug: the owner selects the installation and diff --git a/crates/preloop-runner-server/src/github_breaker.rs b/crates/preloop-runner-server/src/github_breaker.rs index 2468d52e8..df224c4b6 100644 --- a/crates/preloop-runner-server/src/github_breaker.rs +++ b/crates/preloop-runner-server/src/github_breaker.rs @@ -250,6 +250,34 @@ impl GithubBreaker { } } + /// Honour a *primary* rate limit advertised on an otherwise successful + /// response. + /// + /// GitHub sends `x-ratelimit-remaining: 0` plus `x-ratelimit-reset` on the + /// last allowed response. Recording the advertised window here means the + /// caller parks itself until the reset instead of spending the next + /// request on a guaranteed 403. A response without rate headers, or one + /// that still has budget, changes nothing. + pub fn observe_rate_budget(&self, headers: &HeaderMap) { + let remaining = header_str(headers, "x-ratelimit-remaining") + .and_then(|value| value.trim().parse::().ok()); + if !remaining.is_some_and(|remaining| remaining <= 0) { + return; + } + let Some(wait) = retry_after_from_headers(headers) else { + return; + }; + let now = Instant::now(); + let mut inner = self.inner.lock(); + inner.rate_limited = true; + inner.last_error = Some("GitHub primary rate limit exhausted".to_owned()); + Self::open_for( + &mut inner, + now, + wait.clamp(Duration::from_secs(1), Duration::from_secs(3600)), + ); + } + /// Observe a transport-level failure (DNS, TLS, connect, timeout). pub fn observe_transport_error(&self, error: &reqwest::Error) -> GithubFailureKind { let kind = GithubFailureKind::Unavailable; diff --git a/crates/preloop-runner-server/src/lib.rs b/crates/preloop-runner-server/src/lib.rs index a5bdd6cd8..f196164bd 100644 --- a/crates/preloop-runner-server/src/lib.rs +++ b/crates/preloop-runner-server/src/lib.rs @@ -8,6 +8,7 @@ use std::net::SocketAddr; use std::path::PathBuf; use std::sync::Arc; +mod check_run_outbox; pub mod concurrency; pub mod config; pub mod credential_store; diff --git a/crates/preloop-runner-server/src/state.rs b/crates/preloop-runner-server/src/state.rs index 589711423..ccc29bcbc 100644 --- a/crates/preloop-runner-server/src/state.rs +++ b/crates/preloop-runner-server/src/state.rs @@ -721,11 +721,11 @@ pub struct AppState { /// this (see [`wake_waiters`]); only the sampler listens on it. pub sampler_notify: Arc, pub webhook_queue_notify: Arc, - /// Circuit breaker for durable webhook delivery work. Lifecycle check-run - /// updates use a separate breaker so an ingress outage cannot suppress - /// status transitions for runs already in progress. + /// Circuit breaker for durable webhook delivery work. Check-run updates + /// go through the sender's per-installation budgets instead, so an + /// ingress outage cannot suppress status transitions for runs already in + /// progress. pub github_breaker: Arc, - pub github_lifecycle_breaker: Arc, /// Live status published by the delivery watchdog and App webhook health /// monitor. pub webhook_status: Arc, @@ -1470,7 +1470,6 @@ impl AppState { sampler_notify, webhook_queue_notify: Arc::new(Notify::new()), github_breaker: Arc::new(crate::github_breaker::GithubBreaker::default()), - github_lifecycle_breaker: Arc::new(crate::github_breaker::GithubBreaker::default()), webhook_status: Arc::new(crate::webhook_status::WebhookResilienceStatus::default()), webhook_retry_backoff: crate::github::WEBHOOK_RETRY_BACKOFF.to_vec(), observability: preloop_observability::Observability::noop(), diff --git a/crates/preloop-runner-server/tests/broker.rs b/crates/preloop-runner-server/tests/broker.rs index 3c4b930c4..c0fef9dc4 100644 --- a/crates/preloop-runner-server/tests/broker.rs +++ b/crates/preloop-runner-server/tests/broker.rs @@ -263,6 +263,7 @@ jobs: shutdown: CancellationToken::new(), }); crate::github::drain_webhook_queue(&shared).await.unwrap(); + crate::github::drain_check_run_sender(&shared).await; let inner = state.test_tx().await; assert_eq!(inner.runs.len(), 1); // Queue depth is sampled by the 5s sampler, not updated per operation; @@ -668,6 +669,7 @@ jobs: shutdown: CancellationToken::new(), }); crate::github::drain_webhook_queue(&shared).await.unwrap(); + crate::github::drain_check_run_sender(&shared).await; let inner = state.test_tx().await; let (run_id, run) = inner.runs.iter().next().expect("webhook created a run"); @@ -1182,6 +1184,7 @@ jobs: shutdown: CancellationToken::new(), }); crate::github::drain_webhook_queue(&shared).await.unwrap(); + crate::github::drain_check_run_sender(&shared).await; // Verify triggered run let inner = state.test_tx().await; assert_eq!(inner.runs.len(), 1); diff --git a/crates/preloop-runner-server/tests/concurrency.rs b/crates/preloop-runner-server/tests/concurrency.rs index 92a467b39..57320a8e3 100644 --- a/crates/preloop-runner-server/tests/concurrency.rs +++ b/crates/preloop-runner-server/tests/concurrency.rs @@ -2326,10 +2326,12 @@ async fn submit_driven_push_publishes_pr_and_checks_idempotently() { let accepted = submit_push_run(&app, SHA, TREE).await; let run_id = accepted["run_id"].as_str().unwrap().to_owned(); let parsed_run_id: RunId = run_id.parse().unwrap(); - // Check-run creation on submit is detached from the HTTP response, so - // wait for the background reporting task to populate the check run id. + // Submit only records the desired check-run state; the background sender + // creates the check. The harness spawns no sender, so drain it inline + // until the mapping appears. tokio::time::timeout(std::time::Duration::from_secs(5), async { loop { + crate::github::drain_check_run_sender(&state.shared()).await; let ready = { let tx = state.test_tx().await; tx.runs @@ -2673,6 +2675,7 @@ async fn push_intake_reports_no_check_run_for_expandable_placeholders() { // wait for the background reporting task to populate the mapping. tokio::time::timeout(std::time::Duration::from_secs(5), async { loop { + crate::github::drain_check_run_sender(&state.shared()).await; let ready = { let inner = state.test_tx().await; inner @@ -2759,6 +2762,11 @@ async fn dirty_push_sync_verifies_the_branch_head_and_reports_checks_on_the_mate // endpoint matches the whole remaining path. "/repos/owner/repo/commits/*ref", get(|Path(r#ref): Path| async move { + // The wildcard also catches the check-run sender's + // `commits/{sha}/check-runs` dedup lookup: nothing exists yet. + if r#ref.ends_with("/check-runs") { + return Json(json!({"total_count": 0, "check_runs": []})); + } assert_eq!(r#ref, "feat/x", "dirty sync must verify the branch head"); Json(json!({ "sha": MATERIALIZED, @@ -2865,11 +2873,18 @@ async fn dirty_push_sync_verifies_the_branch_head_and_reports_checks_on_the_mate .await; assert_eq!(status, StatusCode::OK, "dirty sync must succeed: {body}"); assert_eq!(pr_creates.load(Ordering::SeqCst), 1, "PR created"); - let check = check_creates - .lock() - .first() - .cloned() - .expect("queued check run"); + // The sync records desired check-run state; the background projector + // queues it and the sender creates the check. The harness spawns no + // sender, so drain it until the POST lands. + tokio::time::timeout(std::time::Duration::from_secs(5), async { + while check_creates.lock().is_empty() { + crate::github::drain_check_run_sender(&state.shared()).await; + tokio::time::sleep(std::time::Duration::from_millis(10)).await; + } + }) + .await + .expect("queued check run"); + let check = check_creates.lock().first().cloned().unwrap(); assert_eq!( check["head_sha"], MATERIALIZED, "checks attach to the materialized head commit, not the base" diff --git a/crates/preloop-runner-server/tests/logs.rs b/crates/preloop-runner-server/tests/logs.rs index 1173ab726..9fe7a3b0d 100644 --- a/crates/preloop-runner-server/tests/logs.rs +++ b/crates/preloop-runner-server/tests/logs.rs @@ -2648,8 +2648,16 @@ async fn cancel_run_completes_github_checks_and_terminal_metadata() { let run_id: RunId = accepted["run_id"].as_str().unwrap().parse().unwrap(); state .test_db_mutate(|tx| { + // A check-run mapping only exists on a run that reports checks + // (webhook or `--push`); stamp both so the projector delivers + // the completion, as it would for a real reporting run. tx.set_job_check_run(run_id, &JobId("build".into()), CHECK_RUN_ID as i64) .unwrap(); + tx.execute( + "UPDATE runs SET reports_check_runs = 1 WHERE run_id = ?1", + [run_id.to_string()], + ) + .unwrap(); }) .await; @@ -2674,6 +2682,8 @@ async fn cancel_run_completes_github_checks_and_terminal_metadata() { .iter() .any(|(id, body)| *id == CHECK_RUN_ID && body["status"] == "completed") { + // The cancel queues the completion; the sender delivers it. + crate::github::drain_check_run_sender(&state.shared()).await; tokio::time::sleep(std::time::Duration::from_millis(10)).await; } }) @@ -2730,8 +2740,15 @@ async fn completed_check_uploads_every_annotation_in_batches_of_fifty() { let job_id = JobId("build".to_owned()); state .test_db_mutate(|tx| { + // A check-run mapping only exists on a run that reports checks; + // stamp both so the projector delivers the completion. tx.set_job_check_run(run_id, &job_id, CHECK_RUN_ID as i64) .unwrap(); + tx.execute( + "UPDATE runs SET reports_check_runs = 1 WHERE run_id = ?1", + [run_id.to_string()], + ) + .unwrap(); }) .await; { @@ -2768,6 +2785,8 @@ async fn completed_check_uploads_every_annotation_in_batches_of_fifty() { .last() .is_some_and(|body| body["status"] == "completed") { + // The cancel queues the completion; the sender delivers it. + crate::github::drain_check_run_sender(&state.shared()).await; tokio::time::sleep(std::time::Duration::from_millis(10)).await; } }) diff --git a/docs/architecture.md b/docs/architecture.md index 1e1a724bf..d0337ca69 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -89,6 +89,37 @@ one node per job). Known gaps and their tradeoffs are tracked in the repository issue tracker. +## Transactional event flow + +Every state transition writes its versioned event to `outbox_events` in the +same database transaction as the run/job mutation. The durable `check-runs` +consumer holds a persisted `consumer_offsets` lease, reads from its bookmark +(`(txid,event_id)` on Postgres and `event_id` on SQLite), and atomically +projects the newest desired state for each `(run_id,job_id)` into +`check_run_updates`. Older versions are ignored. + +```text +run/job transaction -> outbox_events + | + check-runs projector + v + check_run_updates (coalesced) + | + one leased sender + v + GitHub Checks API +``` + +The sender is the only check-run HTTP writer. It leases due rows, renders +the existing check name/summary/details/output at send time, chunks +annotations at GitHub's 50-item limit, and owns token caching, circuit +breaking, rate-limit handling, retries and exponential backoff. A webhook +therefore commits durable state and returns without waiting for GitHub. +Postgres `NOTIFY preloop_events`, the local dirty signal, and a short polling +fallback wake the projector. Outbox pruning is bounded by age only when no +durable consumer exists; otherwise rows at or beyond the slowest consumer +bookmark are retained. + ## Secrets Secrets use `SecretString` in `preloop-gha-protocol`. It redacts `Debug`, diff --git a/docs/control-schema.sql b/docs/control-schema.sql index ef5160bf9..ef400c3c5 100644 --- a/docs/control-schema.sql +++ b/docs/control-schema.sql @@ -657,12 +657,13 @@ CREATE TABLE check_run_updates ( job_id text NOT NULL, installation_id bigint NOT NULL, check_run_id bigint, + version bigint NOT NULL DEFAULT 0, payload jsonb NOT NULL, not_before timestamptz NOT NULL DEFAULT now(), attempts integer NOT NULL DEFAULT 0, leased_until timestamptz, - PRIMARY KEY (run_id, job_id), - FOREIGN KEY (run_id, job_id) REFERENCES jobs(run_id, job_id) ON DELETE CASCADE + lease_owner text, + PRIMARY KEY (run_id, job_id) ); CREATE INDEX check_run_updates_queue ON check_run_updates(installation_id, not_before);