From 0f4b0c95368bb83957c4d648f0b3a92c960d871d Mon Sep 17 00:00:00 2001 From: Huskies Agent Date: Fri, 17 Jul 2026 12:05:52 +0000 Subject: [PATCH] huskies: merge 1185 story Bounded auto-retry for GatesFailed merges before requiring human intervention --- .../merge_failure_retry_subscriber.rs | 570 ++++++++++++++++++ server/src/agents/pool/auto_assign/mod.rs | 6 + server/src/io/watcher/events.rs | 11 + server/src/service/notifications/events.rs | 13 + server/src/service/notifications/format.rs | 62 ++ .../src/service/notifications/io/listener.rs | 34 +- server/src/service/ws/message/convert.rs | 2 + server/src/startup/tick_loop.rs | 12 + 8 files changed, 708 insertions(+), 2 deletions(-) create mode 100644 server/src/agents/pool/auto_assign/merge_failure_retry_subscriber.rs diff --git a/server/src/agents/pool/auto_assign/merge_failure_retry_subscriber.rs b/server/src/agents/pool/auto_assign/merge_failure_retry_subscriber.rs new file mode 100644 index 00000000..a7163e87 --- /dev/null +++ b/server/src/agents/pool/auto_assign/merge_failure_retry_subscriber.rs @@ -0,0 +1,570 @@ +//! TransitionFired subscriber that auto-retries `GatesFailed` merge failures after a delay. +//! +//! Story 1185: ConflictDetected already gets an automatic recovery path via +//! [`super::merge_failure_subscriber`] (mergemaster auto-spawn). `GatesFailed` +//! previously just sat in `Stage::MergeFailure` until a human intervened or the +//! auto-block subscriber ([`super::merge_failure_block_subscriber`]) blocked the +//! story after `merge_failure_block_threshold` consecutive failures. +//! +//! This subscriber schedules a delayed re-trigger of the deterministic +//! server-side merge for `GatesFailed` failures, sharing the same +//! `merge_failure_block_threshold` budget the auto-block subscriber uses as its +//! retry ceiling. Once that many consecutive `GatesFailed` failures have +//! occurred, no further retry is scheduled and the story is left exactly where +//! it is — the untouched auto-block subscriber independently counts the same +//! consecutive-failure stream and transitions the story to `Stage::Blocked` at +//! the same threshold, so behaviour after budget exhaustion is unchanged from +//! today. + +use std::collections::HashMap; +use std::path::{Path, PathBuf}; +use std::sync::Arc; +use std::time::Duration; + +use crate::io::watcher::WatcherEvent; +use crate::pipeline_state::{MergeFailureKind, Stage, Status, StoryId, TransitionFired}; +use crate::slog; +use crate::slog_warn; + +use super::super::AgentPool; + +/// Delay before an auto-retry re-triggers the server-side merge for a +/// `GatesFailed` failure. Gives a moment for transient conditions (e.g. a +/// flaky test) to clear before retrying the exact same commit — retrying +/// instantly would just hammer the same deterministic failure. +const AUTO_RETRY_DELAY: Duration = Duration::from_secs(30); + +/// Reconcile: no-op, matching [`super::merge_failure_block_subscriber`]. +/// +/// The per-story attempt counter is in-memory bookkeeping that cannot be +/// reconstructed from CRDT state alone (only the current stage is stored, not +/// how many times a story has already been auto-retried). The live subscriber +/// reacting to each new transition is sufficient for eventual consistency. +pub(crate) fn reconcile_merge_failure_retry() {} + +/// Spawn a background task that auto-retries `GatesFailed` merge failures. +/// +/// Subscribes to the pipeline transition broadcast channel and tracks a +/// per-story auto-retry attempt counter, bounded by the same +/// `merge_failure_block_threshold` budget the auto-block subscriber uses. +pub(crate) fn spawn_merge_failure_retry_subscriber(pool: Arc, project_root: PathBuf) { + let mut rx = crate::pipeline_state::subscribe_transitions(); + tokio::spawn(async move { + let mut attempts: HashMap = HashMap::new(); + loop { + match rx.recv().await { + Ok(fired) => { + if let Some((attempt, budget)) = + decide_retry(&project_root, &fired, &mut attempts) + { + schedule_auto_retry( + Arc::clone(&pool), + project_root.clone(), + fired.story_id.0.clone(), + attempt, + budget, + ); + } + } + Err(tokio::sync::broadcast::error::RecvError::Lagged(n)) => { + slog_warn!( + "[merge-retry-sub] Subscriber lagged, skipped {n} event(s). \ + Some GatesFailed stories may need a manual retry." + ); + } + Err(tokio::sync::broadcast::error::RecvError::Closed) => break, + } + } + }); +} + +/// Pure decision function: given a fired transition and the running per-story +/// attempt counter, decide whether an auto-retry should be scheduled. +/// +/// Returns `Some((attempt, budget))` when a retry should fire — `attempt` is +/// this retry's 1-based sequence number and `budget` is the shared +/// `merge_failure_block_threshold`. Returns `None` when the story left +/// `MergeFailure`, the failure kind isn't `GatesFailed`, or the budget is +/// exhausted (`attempt >= budget`). +fn decide_retry( + project_root: &Path, + fired: &TransitionFired, + attempts: &mut HashMap, +) -> Option<(u32, u32)> { + if fired.after.status() != Status::MergeFailure { + attempts.remove(&fired.story_id); + return None; + } + let Stage::MergeFailure { kind, .. } = &fired.after else { + return None; + }; + if !matches!(kind, MergeFailureKind::GatesFailed(_)) { + attempts.remove(&fired.story_id); + return None; + } + + let entry = attempts.entry(fired.story_id.clone()).or_insert(0); + *entry += 1; + let attempt = *entry; + + let budget = load_threshold(project_root); + if budget == 0 || attempt >= budget { + slog!( + "[merge-retry-sub] Story '{}' exhausted GatesFailed auto-retry budget \ + ({attempt}/{budget}); parking in MergeFailure for human intervention.", + fired.story_id.0 + ); + return None; + } + + Some((attempt, budget)) +} + +/// Spawn the actual delayed retry as a background task. +/// +/// Kept separate from [`decide_retry`] so the counter/budget decision stays +/// synchronous and unit-testable without waiting on a real timer. +fn schedule_auto_retry( + pool: Arc, + project_root: PathBuf, + story_id: String, + attempt: u32, + budget: u32, +) { + slog!( + "[merge-retry-sub] Story '{story_id}' GatesFailed (attempt {attempt}/{budget}); \ + scheduling auto-retry in {AUTO_RETRY_DELAY:?}." + ); + tokio::spawn(async move { + tokio::time::sleep(AUTO_RETRY_DELAY).await; + perform_auto_retry(&pool, &project_root, &story_id, attempt, budget).await; + }); +} + +/// Re-check the story is still parked in `GatesFailed`, notify chat, and +/// re-trigger the deterministic server-side merge. +/// +/// The guard protects against the story having been fixed manually, blocked, +/// or otherwise moved on while the retry delay elapsed. +async fn perform_auto_retry( + pool: &Arc, + project_root: &Path, + story_id: &str, + attempt: u32, + budget: u32, +) { + let still_gates_failed = matches!( + crate::pipeline_state::read_typed(story_id), + Ok(Some(item)) if matches!( + item.stage, + Stage::MergeFailure { + kind: MergeFailureKind::GatesFailed(_), + .. + } + ) + ); + if !still_gates_failed { + slog!( + "[merge-retry-sub] Story '{story_id}' left GatesFailed before auto-retry \ + ({attempt}/{budget}) fired; skipping." + ); + return; + } + + slog!("[merge-retry-sub] Auto-retrying merge for '{story_id}' (attempt {attempt}/{budget})."); + let _ = pool.watcher_tx.send(WatcherEvent::MergeAutoRetry { + story_id: story_id.to_string(), + attempt, + budget, + }); + pool.trigger_server_side_merge(project_root, story_id); +} + +/// Load the auto-retry budget from project config, falling back to the +/// compiled default. Shares `merge_failure_block_threshold` with the +/// auto-block subscriber (story 1185, AC2). +fn load_threshold(project_root: &Path) -> u32 { + crate::config::ProjectConfig::load(project_root) + .map(|c| c.merge_failure_block_threshold) + .unwrap_or(3) +} + +// ── Tests ────────────────────────────────────────────────────────────────── + +#[cfg(test)] +mod tests { + use super::*; + use crate::pipeline_state::{BranchName, PipelineEvent, Stage, StoryId, TransitionFired}; + use std::num::NonZeroU32; + use std::process::Command; + + fn setup_project(tmp: &tempfile::TempDir) { + let sk = tmp.path().join(".huskies"); + std::fs::create_dir_all(&sk).unwrap(); + std::fs::write(sk.join("project.toml"), "[[agent]]\nname = \"coder\"\n").unwrap(); + } + + fn seed_at_merge(story_id: &str) { + crate::crdt_state::init_for_test(); + crate::db::ensure_content_store(); + crate::db::write_item_with_content( + story_id, + "4_merge", + "---\nname: Test\n---\n", + crate::db::ItemMeta::named("Test"), + ); + } + + fn make_merge_failure_fired(story_id: &str, kind: MergeFailureKind) -> TransitionFired { + TransitionFired { + story_id: StoryId(story_id.to_string()), + before: Stage::Merge { + feature_branch: BranchName("feature/test".to_string()), + commits_ahead: NonZeroU32::new(1).unwrap(), + claim: None, + retries: 0, + server_start_time: None, + }, + after: Stage::MergeFailure { + kind: kind.clone(), + feature_branch: BranchName("feature/test".to_string()), + commits_ahead: NonZeroU32::new(1).unwrap(), + }, + event: PipelineEvent::MergeFailed { kind }, + at: chrono::Utc::now(), + } + } + + fn make_coding_fired(story_id: &str) -> TransitionFired { + TransitionFired { + story_id: StoryId(story_id.to_string()), + before: Stage::MergeFailure { + kind: MergeFailureKind::GatesFailed("error".to_string()), + feature_branch: BranchName("feature/test".to_string()), + commits_ahead: NonZeroU32::new(1).unwrap(), + }, + after: Stage::Coding { + claim: None, + plan: Default::default(), + retries: 0, + }, + event: PipelineEvent::FixupRequested, + at: chrono::Utc::now(), + } + } + + // ── decide_retry: pure decision logic ─────────────────────────────────── + + /// AC1/AC6: a GatesFailed failure below the budget must be scheduled for retry. + #[test] + fn gates_failed_below_budget_schedules_retry() { + let tmp = tempfile::tempdir().unwrap(); + setup_project(&tmp); + let story_id = "1185_retry_basic"; + seed_at_merge(story_id); + + let mut attempts: HashMap = HashMap::new(); + let fired = make_merge_failure_fired( + story_id, + MergeFailureKind::GatesFailed("cargo test failed".to_string()), + ); + + let decision = decide_retry(tmp.path(), &fired, &mut attempts); + assert_eq!( + decision, + Some((1, 3)), + "first GatesFailed failure (default threshold 3) must schedule attempt 1/3" + ); + } + + /// AC2/AC6: once the attempt count reaches the shared threshold, no further + /// retry is scheduled — the story parks in MergeFailure exactly as today + /// (the untouched block subscriber handles the eventual Blocked transition). + #[test] + fn budget_exhaustion_stops_scheduling_retries() { + let tmp = tempfile::tempdir().unwrap(); + setup_project(&tmp); + let story_id = "1185_retry_exhausted"; + seed_at_merge(story_id); + + let mut attempts: HashMap = HashMap::new(); + let kind = MergeFailureKind::GatesFailed("cargo test failed".to_string()); + + // Default threshold is 3: attempts 1 and 2 schedule a retry, attempt 3 + // (== budget) does not. + let fired1 = make_merge_failure_fired(story_id, kind.clone()); + assert_eq!( + decide_retry(tmp.path(), &fired1, &mut attempts), + Some((1, 3)) + ); + + let fired2 = make_merge_failure_fired(story_id, kind.clone()); + assert_eq!( + decide_retry(tmp.path(), &fired2, &mut attempts), + Some((2, 3)) + ); + + let fired3 = make_merge_failure_fired(story_id, kind); + assert_eq!( + decide_retry(tmp.path(), &fired3, &mut attempts), + None, + "budget exhausted at attempt == threshold; must not schedule another retry" + ); + } + + /// AC3/AC6: ConflictDetected, EmptyDiff, NoCommits, and Other must never be + /// auto-retried by this subscriber (ConflictDetected has its own mergemaster + /// auto-spawn path; the rest require human intervention). + #[test] + fn non_gates_failed_kinds_are_not_retried() { + let tmp = tempfile::tempdir().unwrap(); + setup_project(&tmp); + + for (story_id, kind) in [ + ( + "1185_retry_conflict", + MergeFailureKind::ConflictDetected(None), + ), + ("1185_retry_emptydiff", MergeFailureKind::EmptyDiff), + ("1185_retry_nocommits", MergeFailureKind::NoCommits), + ( + "1185_retry_other", + MergeFailureKind::Other("unknown".to_string()), + ), + ] { + seed_at_merge(story_id); + let mut attempts: HashMap = HashMap::new(); + let fired = make_merge_failure_fired(story_id, kind); + assert_eq!( + decide_retry(tmp.path(), &fired, &mut attempts), + None, + "non-GatesFailed kind must not be scheduled for auto-retry" + ); + } + } + + /// Counter resets when the story leaves MergeFailure (e.g. FixupRequested), + /// mirroring the auto-block subscriber's reset behaviour. + #[test] + fn counter_resets_on_non_merge_failure_transition() { + let tmp = tempfile::tempdir().unwrap(); + setup_project(&tmp); + let story_id = "1185_retry_reset"; + seed_at_merge(story_id); + + let mut attempts: HashMap = HashMap::new(); + let kind = MergeFailureKind::GatesFailed("error".to_string()); + + let fired1 = make_merge_failure_fired(story_id, kind.clone()); + decide_retry(tmp.path(), &fired1, &mut attempts); + assert_eq!(attempts.get(&StoryId(story_id.to_string())), Some(&1)); + + let reset_fired = make_coding_fired(story_id); + decide_retry(tmp.path(), &reset_fired, &mut attempts); + assert!( + !attempts.contains_key(&StoryId(story_id.to_string())), + "counter must be cleared after non-MergeFailure transition" + ); + + // A fresh failure after reset must start back at attempt 1. + let fired2 = make_merge_failure_fired(story_id, kind); + assert_eq!( + decide_retry(tmp.path(), &fired2, &mut attempts), + Some((1, 3)) + ); + } + + /// threshold == 0 disables auto-retry entirely, mirroring the auto-block + /// subscriber's "0 disables" convention. + #[test] + fn zero_threshold_disables_auto_retry() { + let tmp = tempfile::tempdir().unwrap(); + let sk = tmp.path().join(".huskies"); + std::fs::create_dir_all(&sk).unwrap(); + // Top-level keys must precede `[[agent]]` — TOML parses anything after + // the array-of-tables header as belonging to that table. + std::fs::write( + sk.join("project.toml"), + "merge_failure_block_threshold = 0\n[[agent]]\nname = \"coder\"\n", + ) + .unwrap(); + let story_id = "1185_retry_zero_threshold"; + seed_at_merge(story_id); + + let mut attempts: HashMap = HashMap::new(); + let fired = + make_merge_failure_fired(story_id, MergeFailureKind::GatesFailed("error".to_string())); + assert_eq!(decide_retry(tmp.path(), &fired, &mut attempts), None); + } + + // ── perform_auto_retry: end-to-end success path ───────────────────────── + + fn init_git_repo(repo: &std::path::Path) { + Command::new("git") + .args(["init"]) + .current_dir(repo) + .output() + .unwrap(); + Command::new("git") + .args(["config", "user.email", "test@test.com"]) + .current_dir(repo) + .output() + .unwrap(); + Command::new("git") + .args(["config", "user.name", "Test"]) + .current_dir(repo) + .output() + .unwrap(); + Command::new("git") + .args(["commit", "--allow-empty", "-m", "init"]) + .current_dir(repo) + .output() + .unwrap(); + } + + /// AC5/AC6: a mid-budget auto-retry that succeeds must drive the story to + /// `Stage::Done` via the same path a manual re-run uses + /// (`trigger_server_side_merge` → `start_merge_agent_work` → + /// `move_story_to_done`) — no 1178-style trap state. + #[tokio::test] + async fn perform_auto_retry_success_completes_the_story() { + use std::fs; + use tempfile::tempdir; + + crate::crdt_state::init_for_test(); + let tmp = tempdir().unwrap(); + let repo = tmp.path(); + init_git_repo(repo); + + let story_id = "1185_retry_success"; + let branch = format!("feature/story-{story_id}"); + Command::new("git") + .args(["checkout", "-b", &branch]) + .current_dir(repo) + .output() + .unwrap(); + fs::write(repo.join("feature.txt"), "feature content").unwrap(); + Command::new("git") + .args(["add", "."]) + .current_dir(repo) + .output() + .unwrap(); + Command::new("git") + .args(["commit", "-m", "add feature"]) + .current_dir(repo) + .output() + .unwrap(); + Command::new("git") + .args(["checkout", "master"]) + .current_dir(repo) + .output() + .unwrap(); + + crate::db::ensure_content_store(); + crate::db::write_item_with_content( + story_id, + "4_merge", + "---\nname: Retry Success Test\n---\n", + crate::db::ItemMeta::named("Retry Success Test"), + ); + crate::agents::lifecycle::transition_to_merge_failure( + story_id, + MergeFailureKind::GatesFailed("cargo test failed".to_string()), + ) + .expect("seed MergeFailure(GatesFailed)"); + + let pool = Arc::new(AgentPool::new_test(3199)); + perform_auto_retry(&pool, repo, story_id, 1, 3).await; + + // start_merge_agent_work transitions MergeFailure -> Merge synchronously + // (transition_merge_failure_to_retry) and then runs the actual squash + // merge as a background task; poll until the story reaches Done (or + // times out). Merge is an expected in-flight state, not a terminal one + // — only stop polling on Done or a re-observed MergeFailure/Blocked. + // Generous deadline: the real git subprocess pipeline (branch + // checkout, worktree add/remove, squash, cherry-pick, source-map + // regen) can be slow under a loaded, parallel `cargo test` run + // (matches the 5s+ headroom used by similar tests in + // `agents/pool/pipeline/merge/tests.rs` and `pipeline/advance/tests_regression.rs`). + let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(30); + loop { + let item = crate::pipeline_state::read_typed(story_id) + .expect("read") + .expect("item"); + if matches!(item.stage, Stage::Done { .. }) { + break; + } + if !matches!(item.stage, Stage::Merge { .. }) { + panic!( + "successful mid-budget auto-retry must complete the story via \ + the manual-rerun path; got {:?}", + item.stage + ); + } + if tokio::time::Instant::now() >= deadline { + panic!( + "auto-retry did not reach Done in time; still {:?}", + item.stage + ); + } + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + } + } + + /// The guard in `perform_auto_retry` must skip re-triggering when the story + /// left GatesFailed while the retry delay was elapsing (e.g. blocked or + /// fixed manually in the meantime). + #[tokio::test] + async fn perform_auto_retry_skips_when_story_left_gates_failed() { + crate::crdt_state::init_for_test(); + let tmp = tempfile::tempdir().unwrap(); + let repo = tmp.path(); + + let story_id = "1185_retry_stale_guard"; + crate::db::ensure_content_store(); + crate::db::write_item_with_content( + story_id, + "4_merge", + "---\nname: Stale Guard Test\n---\n", + crate::db::ItemMeta::named("Stale Guard Test"), + ); + crate::agents::lifecycle::transition_to_merge_failure( + story_id, + MergeFailureKind::GatesFailed("error".to_string()), + ) + .expect("seed MergeFailure(GatesFailed)"); + + // Story moved on (e.g. blocked) before the delayed retry fired. + crate::pipeline_state::apply_transition( + story_id, + PipelineEvent::Block { + reason: "manual block".to_string(), + }, + None, + ) + .expect("block story"); + + let pool = Arc::new(AgentPool::new_test(3198)); + perform_auto_retry(&pool, repo, story_id, 1, 3).await; + + // No new merge job should have been started — the story is still + // Blocked. `transition_to_merge_failure` above already wrote a + // "failed" job entry as part of seeding; the guard skipping the + // retry means that entry must be untouched (in particular, never + // flipped to "running" by `trigger_server_side_merge`). + let item = crate::pipeline_state::read_typed(story_id) + .expect("read") + .expect("item"); + assert!( + matches!(item.stage, Stage::Blocked { .. }), + "guard must skip re-triggering once the story left GatesFailed: {:?}", + item.stage + ); + let job_status = crate::crdt_state::read_merge_job(story_id).map(|j| j.status); + assert_ne!( + job_status.as_deref(), + Some("running"), + "no new merge job should be started when the guard skips the retry" + ); + } +} diff --git a/server/src/agents/pool/auto_assign/mod.rs b/server/src/agents/pool/auto_assign/mod.rs index 0082b790..416e9b6b 100644 --- a/server/src/agents/pool/auto_assign/mod.rs +++ b/server/src/agents/pool/auto_assign/mod.rs @@ -6,6 +6,8 @@ mod backlog; mod merge; /// TransitionFired subscriber that auto-blocks stories after N consecutive MergeFailure transitions. pub(crate) mod merge_failure_block_subscriber; +/// TransitionFired subscriber that auto-retries GatesFailed merge failures after a delay. +pub(crate) mod merge_failure_retry_subscriber; /// TransitionFired subscriber that auto-spawns mergemaster on ConflictDetected merge failures. pub(crate) mod merge_failure_subscriber; mod pipeline; @@ -22,6 +24,10 @@ pub(crate) use merge_failure_block_subscriber::reconcile_merge_failure_block; /// Re-export for `startup::tick_loop`. pub(crate) use merge_failure_block_subscriber::spawn_merge_failure_block_subscriber; /// Re-export for `startup::tick_loop`. +pub(crate) use merge_failure_retry_subscriber::reconcile_merge_failure_retry; +/// Re-export for `startup::tick_loop`. +pub(crate) use merge_failure_retry_subscriber::spawn_merge_failure_retry_subscriber; +/// Re-export for `startup::tick_loop`. pub(crate) use merge_failure_subscriber::reconcile_merge_failure; /// Re-export for `startup::tick_loop`. pub(crate) use merge_failure_subscriber::spawn_merge_failure_subscriber; diff --git a/server/src/io/watcher/events.rs b/server/src/io/watcher/events.rs index 9c441ee5..3d526a2d 100644 --- a/server/src/io/watcher/events.rs +++ b/server/src/io/watcher/events.rs @@ -100,4 +100,15 @@ pub enum WatcherEvent { /// Human-readable item name. name: String, }, + /// A `GatesFailed` merge failure is being automatically retried after a + /// delay, without human intervention (story 1185). + /// Triggers a status notification to configured chat rooms. + MergeAutoRetry { + /// Work item ID (e.g. `"42_story_my_feature"`). + story_id: String, + /// This retry's attempt number (1-based). + attempt: u32, + /// Total auto-retry budget shared with the auto-block threshold. + budget: u32, + }, } diff --git a/server/src/service/notifications/events.rs b/server/src/service/notifications/events.rs index d839b89d..c9d131cc 100644 --- a/server/src/service/notifications/events.rs +++ b/server/src/service/notifications/events.rs @@ -28,6 +28,8 @@ pub enum EventAction { }, /// Post a new-item-created notification. NewItemCreated, + /// Post a merge-auto-retry notification naming the attempt and budget. + MergeAutoRetry, /// Log server-side only; do not post to chat (e.g. hard rate-limit blocks). LogOnly, /// Reload the project configuration. @@ -54,6 +56,7 @@ pub fn classify(event: &WatcherEvent) -> EventAction { EventAction::AgentCompleted { success: *success } } WatcherEvent::NewItemCreated { .. } => EventAction::NewItemCreated, + WatcherEvent::MergeAutoRetry { .. } => EventAction::MergeAutoRetry, _ => EventAction::Skip, } } @@ -191,4 +194,14 @@ mod tests { }; assert_eq!(classify(&event), EventAction::NewItemCreated); } + + #[test] + fn merge_auto_retry_is_classified_correctly() { + let event = WatcherEvent::MergeAutoRetry { + story_id: "1_story_foo".to_string(), + attempt: 1, + budget: 3, + }; + assert_eq!(classify(&event), EventAction::MergeAutoRetry); + } } diff --git a/server/src/service/notifications/format.rs b/server/src/service/notifications/format.rs index 5c4eb33f..69b4b1c6 100644 --- a/server/src/service/notifications/format.rs +++ b/server/src/service/notifications/format.rs @@ -240,6 +240,38 @@ pub fn format_new_item_notification( (plain, html) } +/// Format a merge-auto-retry notification message. +/// +/// Sent when a `GatesFailed` merge failure is automatically retried after a +/// delay (story 1185). Returns `(plain_text, html)` suitable for +/// `ChatTransport::send_message`. +pub fn format_merge_auto_retry_notification( + item_id: &str, + story_name: &str, + attempt: u32, + budget: u32, +) -> (String, String) { + let number = extract_item_number(item_id).unwrap_or(item_id); + let effective_name = if story_name.is_empty() { + None + } else { + Some(story_name) + }; + let name_plain = effective_name.map(|n| format!("{n} ")).unwrap_or_default(); + let name_html = effective_name + .map(|n| format!("{n} ")) + .unwrap_or_default(); + + let plain = format!( + "\u{1f504} #{number} {name_plain}\u{2014} auto-retrying merge (attempt {attempt}/{budget})" + ); + let html = format!( + "\u{1f504} #{number} {name_html}\u{2014} auto-retrying merge \ + (attempt {attempt}/{budget})" + ); + (plain, html) +} + /// Maximum number of trailing gate-output lines included in a merge-failure /// chat notification. /// @@ -595,6 +627,36 @@ mod tests { assert_eq!(plain, "\u{1F916} #42 \u{2014} coder-1 started"); } + // ── format_merge_auto_retry_notification ────────────────────────────────── + + #[test] + fn format_merge_auto_retry_notification_with_story_name() { + let (plain, html) = + format_merge_auto_retry_notification("42_story_foo", "My Feature", 1, 3); + assert_eq!( + plain, + "\u{1f504} #42 My Feature \u{2014} auto-retrying merge (attempt 1/3)" + ); + assert_eq!( + html, + "\u{1f504} #42 My Feature \u{2014} auto-retrying merge \ + (attempt 1/3)" + ); + } + + #[test] + fn format_merge_auto_retry_notification_falls_back_to_number() { + let (plain, html) = format_merge_auto_retry_notification("42_story_foo", "", 2, 3); + assert_eq!( + plain, + "\u{1f504} #42 \u{2014} auto-retrying merge (attempt 2/3)" + ); + assert_eq!( + html, + "\u{1f504} #42 \u{2014} auto-retrying merge (attempt 2/3)" + ); + } + // ── truncate_gate_output ────────────────────────────────────────────────── #[test] diff --git a/server/src/service/notifications/io/listener.rs b/server/src/service/notifications/io/listener.rs index a3480b32..7468b65a 100644 --- a/server/src/service/notifications/io/listener.rs +++ b/server/src/service/notifications/io/listener.rs @@ -16,8 +16,9 @@ use super::super::filter::{AGENT_EVENT_DEBOUNCE, should_send_rate_limit}; use super::super::format::{ MERGE_FAILURE_TAIL_LINES, format_agent_completed_notification, format_agent_started_notification, format_blocked_notification, format_error_notification, - format_new_item_notification, format_oauth_account_swapped, format_oauth_accounts_exhausted, - format_rate_limit_notification, truncate_gate_output, + format_merge_auto_retry_notification, format_new_item_notification, + format_oauth_account_swapped, format_oauth_accounts_exhausted, format_rate_limit_notification, + truncate_gate_output, }; use super::super::route::rooms_for_notification; use super::{find_story_name_any_stage, read_story_name}; @@ -295,6 +296,35 @@ pub fn spawn_notification_listener( } } } + EventAction::MergeAutoRetry => { + if !config.status_push_enabled { + continue; + } + let WatcherEvent::MergeAutoRetry { + ref story_id, + attempt, + budget, + } = event + else { + continue; + }; + let story_name = find_story_name_any_stage(&project_root, story_id); + let (plain, html) = format_merge_auto_retry_notification( + story_id, + &story_name, + attempt, + budget, + ); + slog!("[bot] Sending merge-auto-retry notification: {plain}"); + for room_id in &rooms_for_notification(&get_room_ids) { + if let Err(e) = transport.send_message(room_id, &plain, &html).await { + slog!( + "[bot] Failed to send merge-auto-retry notification \ + to {room_id}: {e}" + ); + } + } + } EventAction::LogOnly => { // Hard-block: log server-side for debugging; do NOT post to chat. // Hard-block auto-resume is normal operation — the status command diff --git a/server/src/service/ws/message/convert.rs b/server/src/service/ws/message/convert.rs index 6330435a..96c79216 100644 --- a/server/src/service/ws/message/convert.rs +++ b/server/src/service/ws/message/convert.rs @@ -39,6 +39,8 @@ pub fn watcher_event_to_response(e: WatcherEvent) -> Option { WatcherEvent::AgentCompleted { .. } => None, // Creation notifications are forwarded to chat transports only; no WebSocket message. WatcherEvent::NewItemCreated { .. } => None, + // Merge-auto-retry notifications are forwarded to chat transports only; no WebSocket message. + WatcherEvent::MergeAutoRetry { .. } => None, } } diff --git a/server/src/startup/tick_loop.rs b/server/src/startup/tick_loop.rs index 8608afb5..4394697d 100644 --- a/server/src/startup/tick_loop.rs +++ b/server/src/startup/tick_loop.rs @@ -98,6 +98,15 @@ pub(crate) fn spawn_event_bridges( root.clone(), ); + // GatesFailed auto-retry subscriber: re-triggers the server-side merge + // after a delay for GatesFailed failures, bounded by the same + // merge_failure_block_threshold budget the auto-block subscriber above + // uses (story 1185). + crate::agents::pool::auto_assign::spawn_merge_failure_retry_subscriber( + Arc::clone(&agents), + root.clone(), + ); + // Content-store GC subscriber: purges all ContentKey::* entries for a // story when it reaches a terminal stage, preventing zombie entries from // accumulating in the process heap (story 996). @@ -542,6 +551,9 @@ pub(crate) async fn run_reconcile_pass( // Merge-block: no-op (in-memory counter cannot be reconstructed from CRDT). crate::agents::pool::auto_assign::reconcile_merge_failure_block(); + // Merge-retry: no-op (in-memory attempt counter cannot be reconstructed from CRDT). + crate::agents::pool::auto_assign::reconcile_merge_failure_retry(); + // Audit-log: no-op (historical replay would produce misleading entries). crate::pipeline_state::reconcile_audit_log(); }