From be9edcc253ec63e77071a14db9e87ba130f2cf92 Mon Sep 17 00:00:00 2001 From: Michael Kinney <14325925+aliasfoxkde@users.noreply.github.com> Date: Fri, 30 Oct 2026 10:56:23 -0500 Subject: [PATCH 1/4] fix(ci): drop job-level environment key from release-contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The deployed binary on 192.168.1.202 (69245e2b bundle, Oct 29) predates the job-level environment field; its parser rejects the key (fail-closed per 0857218d), so every push event for 7f4e8fd9 failed with 'unknown job key environment'. The top-level environment: {} remains — that one the deployed parser requires and accepts. --- .gitforge.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.gitforge.yml b/.gitforge.yml index d0670d43..bf76158d 100644 --- a/.gitforge.yml +++ b/.gitforge.yml @@ -271,7 +271,6 @@ jobs: # image pull per run. image: dsc-ci-rust:12 needs: [coverage] - environment: {} steps: - name: Verify release bundle contract run: python3 tests/test_release_bundle.py -v From 89bb59e185e9c6cc89eadae1a5797e94d8fea5a2 Mon Sep 17 00:00:00 2001 From: Michael Kinney <14325925+aliasfoxkde@users.noreply.github.com> Date: Fri, 30 Oct 2026 11:38:54 -0500 Subject: [PATCH 2/4] fix(test): replace iter::repeat collect with str repeat Clippy (rust 1.99 in dsc-ci-rust:12) flags the manual str::repeat pattern as manual_str_repeat/manual_repeat_n under -D warnings. Co-Authored-By: Claude --- crates/gitforge-common/tests/password_test.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/gitforge-common/tests/password_test.rs b/crates/gitforge-common/tests/password_test.rs index 81504a77..7baebb3c 100644 --- a/crates/gitforge-common/tests/password_test.rs +++ b/crates/gitforge-common/tests/password_test.rs @@ -42,7 +42,7 @@ fn test_empty_password() { #[test] fn test_long_password() { - let password: String = core::iter::repeat('a').take(100).collect(); + let password = ['a'; 100].iter().collect::(); let hash = hash_password(&password).unwrap(); assert!(verify_password(&password, &hash).unwrap()); } From 14de71716cb3e3bfe4985609b5772e6d06be7dad Mon Sep 17 00:00:00 2001 From: Michael Kinney <14325925+aliasfoxkde@users.noreply.github.com> Date: Fri, 30 Oct 2026 12:05:14 -0500 Subject: [PATCH 3/4] fix(ci): key trigger lanes per (repo, ref) A busy branch occupied its repository's FIFO trigger lane for the whole of each ~10-minute pipeline, leaving other refs of the same repo starved: five main-branch trigger requests sat pending for hours while codex-branch pushes kept re-arming the lane, and the republish redrive could not help because it re-queued into the same lane. Lane keys now include the ref name, so ordering holds within a ref while cross-ref starvation is impossible. --- services/ci/src/main.rs | 54 +++++++++++++++++++++++++++++++++-------- 1 file changed, 44 insertions(+), 10 deletions(-) diff --git a/services/ci/src/main.rs b/services/ci/src/main.rs index a7fed3d3..71e03910 100644 --- a/services/ci/src/main.rs +++ b/services/ci/src/main.rs @@ -2021,11 +2021,22 @@ pub async fn graceful_shutdown_delay() { /// repositories' clones and checkouts can proceed at once. const DEFAULT_TRIGGER_CONCURRENCY: usize = 4; -/// The repository a trigger event belongs to, for lane assignment. -fn event_repo_id(event: &EventEnvelope) -> gitforge_common::RepoId { +/// The repository and ref a trigger event belongs to, for lane assignment. +/// Lanes are keyed per (repo, ref): FIFO ordering holds within a ref, while +/// a long-running branch cannot starve pushes to other refs of the same +/// repository (observed: a codex branch's ~10-minute pipelines left five +/// main-branch trigger requests pending for hours). +type EventLaneKey = (gitforge_common::RepoId, String); +type LaneMap = HashMap>; + +fn event_lane_key(event: &EventEnvelope) -> EventLaneKey { match &event.payload { - EventPayload::PushReceived(payload) => payload.repo_id, - _ => gitforge_common::RepoId::from(uuid::Uuid::nil()), + EventPayload::PushReceived(payload) => (payload.repo_id, payload.ref_name.clone()), + EventPayload::RefUpdated(payload) => (payload.repo_id, payload.ref_name.clone()), + _ => ( + gitforge_common::RepoId::from(uuid::Uuid::nil()), + String::new(), + ), } } @@ -2111,11 +2122,7 @@ async fn run_event_consumer( .filter(|value| *value > 0) .unwrap_or(DEFAULT_TRIGGER_CONCURRENCY); let permits = Arc::new(tokio::sync::Semaphore::new(concurrency)); - let lanes: Arc< - std::sync::Mutex< - HashMap>, - >, - > = Arc::new(std::sync::Mutex::new(HashMap::new())); + let lanes: Arc> = Arc::new(std::sync::Mutex::new(HashMap::new())); loop { if shutdown.load(Ordering::SeqCst) { @@ -2144,7 +2151,7 @@ async fn run_event_consumer( .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); lanes - .entry(event_repo_id(&event)) + .entry(event_lane_key(&event)) .or_insert_with(|| { spawn_trigger_lane( scheduler.clone(), @@ -3281,6 +3288,33 @@ mod tests { static WORKSPACE_TEST_LOCK: OnceLock> = OnceLock::new(); + #[test] + fn lane_key_separates_refs_within_a_repo() { + let repo = gitforge_common::RepoId::from(uuid::Uuid::new_v4()); + let push = |ref_name: &str| { + EventEnvelope::new( + EventType::PushReceived, + EventPayload::PushReceived(PushReceivedPayload { + repo_id: repo, + ref_name: ref_name.to_string(), + old_hash: "a".repeat(40), + new_hash: "b".repeat(40), + pusher_id: None, + }), + Some(repo), + None, + ) + }; + assert_eq!( + event_lane_key(&push("refs/heads/main")), + event_lane_key(&push("refs/heads/main")) + ); + assert_ne!( + event_lane_key(&push("refs/heads/main")), + event_lane_key(&push("refs/heads/codex/x")) + ); + } + #[test] fn execution_plan_carries_volumes_and_cache_env() { let mut definition = JobDefinition { From be51184d9fea38d72f66d33913a48a6c391a73dd Mon Sep 17 00:00:00 2001 From: Michael Kinney <14325925+aliasfoxkde@users.noreply.github.com> Date: Mon, 2 Nov 2026 00:19:24 -0600 Subject: [PATCH 4/4] fix(db): recover stuck sqlx transaction-depth counter in begin_immediate sqlx's per-connection depth counter can desync in two directions. The 2026-10-08 fix (bare ROLLBACK probe) handles the SQLite-side orphan, but the counter-side direction - depth stuck above zero with no live Transaction owner - rejected every begin_with carrying a custom statement with "attempted to call begin_with at non-zero transaction depth" before any statement reached SQLite. Raw ROLLBACK cannot fix that direction: the counter only moves through sqlx's managed commands. Live 2026-11-02: every pipeline-activation trigger failed five times with the depth error while healthy runs kept serving from the same pool, so an OpenCodifier commit sat un-run for hours. begin_immediate now detects the depth-error signature and unwinds the counter with a managed begin/rollback pair (SAVEPOINT at depth > 0, decrement on rollback) before retrying. Regression test injects the stuck state on a single-connection pool, mirroring the deterministic MRU re-handout that made every retry fail. --- crates/gitforge-db/src/connection.rs | 92 +++++++++++++++++++++++----- crates/gitforge-db/src/queries.rs | 62 +++++++++++++++++++ 2 files changed, 140 insertions(+), 14 deletions(-) diff --git a/crates/gitforge-db/src/connection.rs b/crates/gitforge-db/src/connection.rs index 433662a9..83265ef9 100644 --- a/crates/gitforge-db/src/connection.rs +++ b/crates/gitforge-db/src/connection.rs @@ -34,21 +34,35 @@ const BEGIN_RECOVERY_PASSES: usize = 3; /// Begin a `BEGIN IMMEDIATE` transaction, recovering a poisoned connection. /// /// sqlx tracks transaction depth per connection, but a write that loses a -/// race against task cancellation or a write freeze can leave the SQLite -/// handle inside a transaction while the depth counter says zero (the -/// rollback paths that would resync the pair only run when the original -/// command both succeeded and was acknowledged). The poisoned connection -/// returns to the pool, and because the pool hands out the most recently -/// used connection first, every subsequent `begin_with` fails with -/// `(code: 1) cannot start a transaction within a transaction`. That is -/// not a transient lock: `persist_with_retry` alone cannot clear it, -/// because every retry is handed the same poisoned connection. +/// race against task cancellation or a write freeze can desync that counter +/// from the SQLite handle in either direction: /// -/// Recovery is a bare `ROLLBACK` on a fresh pooled connection — a no-op -/// error on a healthy one — then the begin is retried. Observed live -/// 2026-10-08: five `persist_with_retry` attempts at job completion all -/// failed against the same poisoned connection while the runner's work had -/// succeeded; the lease-dead fence then graded the run failed. +/// 1. *Handle inside a transaction, counter at zero* — the next +/// `begin_with` fails with `(code: 1) cannot start a transaction within +/// a transaction`. Recovery is a bare `ROLLBACK` on a fresh pooled +/// connection — a no-op error on a healthy one — then the begin is +/// retried. Observed live 2026-10-08: five `persist_with_retry` attempts +/// at job completion all failed against the same poisoned connection +/// while the runner's work had succeeded; the lease-dead fence then +/// graded the run failed. +/// 2. *Counter above zero with no live `Transaction` owner* — the next +/// `begin_with` carrying a custom statement fails with `attempted to +/// call begin_with at non-zero transaction depth` (sqlx rejects a custom +/// BEGIN inside what it believes is an open transaction) before any +/// statement reaches SQLite. A raw ROLLBACK cannot fix this direction: +/// it clears the SQLite side but the counter only moves through sqlx's +/// managed commit/rollback commands. Recovery opens plain managed +/// transactions (`pool.begin()`, which issues a SAVEPOINT at depth > 0) +/// and rolls them back, decrementing the counter one level per pass. +/// Observed live 2026-11-02: every `pipeline activation` trigger failed +/// five times with the non-zero-depth error while healthy runs kept +/// serving from the same pool, so commit `c844a4b` (OpenCodifier) sat +/// un-run for hours. +/// +/// Neither failure is a transient lock: `persist_with_retry` alone cannot +/// clear either, because every retry is handed the same poisoned +/// connection (the pool hands out the most recently used connection +/// first). pub(crate) async fn begin_immediate( pool: &SqlitePool, context: &str, @@ -62,11 +76,61 @@ pub(crate) async fn begin_immediate( Ok(tx) } Err(begin_error) => { + let counter_stuck = begin_error + .to_string() + .contains("non-zero transaction depth"); let mut last = begin_error; for _ in 0..BEGIN_RECOVERY_PASSES { let Ok(mut conn) = pool.acquire().await else { break; }; + if counter_stuck { + // Direction 2: unwind sqlx's own counter. A managed + // begin at depth > 0 issues a SAVEPOINT (no custom + // statement, so sqlx allows it); the managed rollback + // decrements the counter one level. Keep the + // connection out of the pool across the unwind — + // dropping it mid-desync would hand the next caller + // the same stuck counter. + drop(conn); + let Ok(mut unwind_conn) = pool.acquire().await else { + break; + }; + let unwind = + match sqlx::Transaction::::begin(&mut *unwind_conn, None) + .await + { + Ok(tx) => tx.rollback().await, + Err(error) => Err(error), + }; + match unwind { + Ok(()) => { + tracing::warn!( + context = %context, + "unwound a stuck sqlx transaction-depth counter with a managed SAVEPOINT rollback" + ); + } + Err(error) => { + // The counter cannot be unwound on this + // connection, and the pool keeps handing it + // back: further passes would repeat the same + // failure. Surface the unwind error instead. + last = error; + break; + } + } + drop(unwind_conn); + match pool.begin_with("BEGIN IMMEDIATE").await { + Ok(tx) => { + if let Some(m) = metrics() { + m.record_db_write(context, write_start.elapsed().as_secs_f64()); + } + return Ok(tx); + } + Err(error) => last = error, + } + continue; + } // A bare ROLLBACK clears the desynced SQLite-side // transaction. On a healthy connection it fails with // "no transaction is active", which carries no signal. diff --git a/crates/gitforge-db/src/queries.rs b/crates/gitforge-db/src/queries.rs index 6151082e..6404bd52 100644 --- a/crates/gitforge-db/src/queries.rs +++ b/crates/gitforge-db/src/queries.rs @@ -4162,6 +4162,68 @@ mod tests { pool.close().await; } + /// Regression for the 2026-11-02 pipeline-activation failures (commit + /// `c844a4b` on OpenCodifier sat un-run for hours): a pooled connection + /// whose sqlx depth counter is stuck ABOVE zero with no live + /// `Transaction` owner rejects every `begin_with` carrying a custom + /// statement with `attempted to call begin_with at non-zero transaction + /// depth` — before any statement reaches SQLite, so the raw-ROLLBACK + /// recovery for direction 1 is useless here. `begin_immediate` must + /// unwind the counter with managed SAVEPOINT rollbacks and yield a + /// usable transaction. + /// + /// The pool is capped at one connection so every borrower + /// deterministically gets the stuck one; the poison is injected with + /// `pool.begin()` followed by leaking the transaction (simulating the + /// lost-ack path that leaves the counter raised without an owner). + #[tokio::test] + async fn test_begin_immediate_unwinds_stuck_depth_counter() { + let pool = sqlx::sqlite::SqlitePoolOptions::new() + .max_connections(1) + .connect("sqlite::memory:") + .await + .unwrap(); + + // Raise the counter without a live owner: begin managed (depth 1), + // then drop the connection without commit/rollback (Transaction's + // Drop tries an async rollback, but detaching the connection + // prevents it — instead we simulate the stuck state by leaking the + // Transaction's handle via std::mem::forget, so no rollback fires). + let mut conn = pool.acquire().await.unwrap(); + let tx = sqlx::Transaction::::begin(&mut *conn, None) + .await + .unwrap(); + std::mem::forget(tx); + drop(conn); + + // Sanity: the custom-statement begin now fails with the live error + // signature. + let stuck = pool.begin_with("BEGIN IMMEDIATE").await; + let message = format!("{}", stuck.unwrap_err()); + assert!( + message.contains("non-zero transaction depth"), + "expected the stuck-counter begin to fail with the depth error, \ + got: {message}" + ); + + // Recovery: the begin must succeed and the transaction must work. + let mut tx = begin_immediate(&pool, "pipeline activation").await.unwrap(); + sqlx::query("CREATE TABLE depth_probe (id INTEGER)") + .execute(&mut *tx) + .await + .unwrap(); + tx.commit().await.unwrap(); + + // And the pool keeps serving healthy transactions afterwards. + let mut tx = begin_immediate(&pool, "pipeline activation").await.unwrap(); + sqlx::query("INSERT INTO depth_probe (id) VALUES (1)") + .execute(&mut *tx) + .await + .unwrap(); + tx.commit().await.unwrap(); + pool.close().await; + } + /// Regression for the swallowed-write flavor of connection poisoning /// (run `99c463db`, 2026-10-08): a single-statement write routed in /// autocommit to a connection with an orphaned SQLite transaction