storkit: merge 425_story_chat_notification_when_a_story_blocks_with_reason
This commit is contained in:
@@ -76,12 +76,17 @@ impl AgentPool {
|
|||||||
.join(".storkit/work")
|
.join(".storkit/work")
|
||||||
.join(stage_dir)
|
.join(stage_dir)
|
||||||
.join(format!("{story_id}.md"));
|
.join(format!("{story_id}.md"));
|
||||||
|
let empty_diff_reason = "Feature branch has no code changes — the coder agent \
|
||||||
|
did not produce any commits.";
|
||||||
let _ = crate::io::story_metadata::write_merge_failure(
|
let _ = crate::io::story_metadata::write_merge_failure(
|
||||||
&story_path,
|
&story_path,
|
||||||
"Feature branch has no code changes — the coder agent \
|
empty_diff_reason,
|
||||||
did not produce any commits.",
|
|
||||||
);
|
);
|
||||||
let _ = crate::io::story_metadata::write_blocked(&story_path);
|
let _ = crate::io::story_metadata::write_blocked(&story_path);
|
||||||
|
let _ = self.watcher_tx.send(crate::io::watcher::WatcherEvent::StoryBlocked {
|
||||||
|
story_id: story_id.to_string(),
|
||||||
|
reason: empty_diff_reason.to_string(),
|
||||||
|
});
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -122,8 +122,12 @@ impl AgentPool {
|
|||||||
let story_path = project_root
|
let story_path = project_root
|
||||||
.join(".storkit/work/2_current")
|
.join(".storkit/work/2_current")
|
||||||
.join(format!("{story_id}.md"));
|
.join(format!("{story_id}.md"));
|
||||||
if should_block_story(&story_path, config.max_retries, story_id, "coder") {
|
if let Some(reason) = should_block_story(&story_path, config.max_retries, story_id, "coder") {
|
||||||
// Story has exceeded retry limit — do not restart.
|
// Story has exceeded retry limit — do not restart.
|
||||||
|
let _ = self.watcher_tx.send(WatcherEvent::StoryBlocked {
|
||||||
|
story_id: story_id.to_string(),
|
||||||
|
reason,
|
||||||
|
});
|
||||||
} else {
|
} else {
|
||||||
slog!(
|
slog!(
|
||||||
"[pipeline] Coder '{agent_name}' failed gates for '{story_id}'. Restarting."
|
"[pipeline] Coder '{agent_name}' failed gates for '{story_id}'. Restarting."
|
||||||
@@ -221,8 +225,12 @@ impl AgentPool {
|
|||||||
let story_path = project_root
|
let story_path = project_root
|
||||||
.join(".storkit/work/3_qa")
|
.join(".storkit/work/3_qa")
|
||||||
.join(format!("{story_id}.md"));
|
.join(format!("{story_id}.md"));
|
||||||
if should_block_story(&story_path, config.max_retries, story_id, "qa-coverage") {
|
if let Some(reason) = should_block_story(&story_path, config.max_retries, story_id, "qa-coverage") {
|
||||||
// Story has exceeded retry limit — do not restart.
|
// Story has exceeded retry limit — do not restart.
|
||||||
|
let _ = self.watcher_tx.send(WatcherEvent::StoryBlocked {
|
||||||
|
story_id: story_id.to_string(),
|
||||||
|
reason,
|
||||||
|
});
|
||||||
} else {
|
} else {
|
||||||
slog!(
|
slog!(
|
||||||
"[pipeline] QA coverage gate failed for '{story_id}'. Restarting QA."
|
"[pipeline] QA coverage gate failed for '{story_id}'. Restarting QA."
|
||||||
@@ -245,8 +253,12 @@ impl AgentPool {
|
|||||||
let story_path = project_root
|
let story_path = project_root
|
||||||
.join(".storkit/work/3_qa")
|
.join(".storkit/work/3_qa")
|
||||||
.join(format!("{story_id}.md"));
|
.join(format!("{story_id}.md"));
|
||||||
if should_block_story(&story_path, config.max_retries, story_id, "qa") {
|
if let Some(reason) = should_block_story(&story_path, config.max_retries, story_id, "qa") {
|
||||||
// Story has exceeded retry limit — do not restart.
|
// Story has exceeded retry limit — do not restart.
|
||||||
|
let _ = self.watcher_tx.send(WatcherEvent::StoryBlocked {
|
||||||
|
story_id: story_id.to_string(),
|
||||||
|
reason,
|
||||||
|
});
|
||||||
} else {
|
} else {
|
||||||
slog!("[pipeline] QA failed gates for '{story_id}'. Restarting.");
|
slog!("[pipeline] QA failed gates for '{story_id}'. Restarting.");
|
||||||
let context = format!(
|
let context = format!(
|
||||||
@@ -321,8 +333,12 @@ impl AgentPool {
|
|||||||
let story_path = project_root
|
let story_path = project_root
|
||||||
.join(".storkit/work/4_merge")
|
.join(".storkit/work/4_merge")
|
||||||
.join(format!("{story_id}.md"));
|
.join(format!("{story_id}.md"));
|
||||||
if should_block_story(&story_path, config.max_retries, story_id, "mergemaster") {
|
if let Some(reason) = should_block_story(&story_path, config.max_retries, story_id, "mergemaster") {
|
||||||
// Story has exceeded retry limit — do not restart.
|
// Story has exceeded retry limit — do not restart.
|
||||||
|
let _ = self.watcher_tx.send(WatcherEvent::StoryBlocked {
|
||||||
|
story_id: story_id.to_string(),
|
||||||
|
reason,
|
||||||
|
});
|
||||||
} else {
|
} else {
|
||||||
slog!(
|
slog!(
|
||||||
"[pipeline] Post-merge tests failed for '{story_id}'. Restarting mergemaster."
|
"[pipeline] Post-merge tests failed for '{story_id}'. Restarting mergemaster."
|
||||||
@@ -830,15 +846,15 @@ fn spawn_pipeline_advance(
|
|||||||
|
|
||||||
/// Increment retry_count and block the story if it exceeds `max_retries`.
|
/// Increment retry_count and block the story if it exceeds `max_retries`.
|
||||||
///
|
///
|
||||||
/// Returns `true` if the story is now blocked (caller should NOT restart the agent).
|
/// Returns `Some(reason)` if the story is now blocked (caller should NOT restart the agent).
|
||||||
/// Returns `false` if the story may be retried.
|
/// Returns `None` if the story may be retried.
|
||||||
/// When `max_retries` is 0, retry limits are disabled.
|
/// When `max_retries` is 0, retry limits are disabled.
|
||||||
fn should_block_story(story_path: &Path, max_retries: u32, story_id: &str, stage_label: &str) -> bool {
|
fn should_block_story(story_path: &Path, max_retries: u32, story_id: &str, stage_label: &str) -> Option<String> {
|
||||||
use crate::io::story_metadata::{increment_retry_count, write_blocked};
|
use crate::io::story_metadata::{increment_retry_count, write_blocked};
|
||||||
|
|
||||||
if max_retries == 0 {
|
if max_retries == 0 {
|
||||||
// Retry limits disabled.
|
// Retry limits disabled.
|
||||||
return false;
|
return None;
|
||||||
}
|
}
|
||||||
|
|
||||||
match increment_retry_count(story_path) {
|
match increment_retry_count(story_path) {
|
||||||
@@ -851,17 +867,19 @@ fn should_block_story(story_path: &Path, max_retries: u32, story_id: &str, stage
|
|||||||
if let Err(e) = write_blocked(story_path) {
|
if let Err(e) = write_blocked(story_path) {
|
||||||
slog_error!("[pipeline] Failed to write blocked flag for '{story_id}': {e}");
|
slog_error!("[pipeline] Failed to write blocked flag for '{story_id}': {e}");
|
||||||
}
|
}
|
||||||
true
|
Some(format!(
|
||||||
|
"Retry limit exceeded ({new_count}/{max_retries}) at {stage_label} stage"
|
||||||
|
))
|
||||||
} else {
|
} else {
|
||||||
slog!(
|
slog!(
|
||||||
"[pipeline] Story '{story_id}' retry {new_count}/{max_retries} at {stage_label} stage."
|
"[pipeline] Story '{story_id}' retry {new_count}/{max_retries} at {stage_label} stage."
|
||||||
);
|
);
|
||||||
false
|
None
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
slog_error!("[pipeline] Failed to increment retry_count for '{story_id}': {e}");
|
slog_error!("[pipeline] Failed to increment retry_count for '{story_id}': {e}");
|
||||||
false // Don't block on error — allow retry.
|
None // Don't block on error — allow retry.
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -115,6 +115,24 @@ fn find_story_name_any_stage(project_root: &Path, item_id: &str) -> Option<Strin
|
|||||||
None
|
None
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Format a blocked-story notification message.
|
||||||
|
///
|
||||||
|
/// Returns `(plain_text, html)` suitable for `ChatTransport::send_message`.
|
||||||
|
pub fn format_blocked_notification(
|
||||||
|
item_id: &str,
|
||||||
|
story_name: Option<&str>,
|
||||||
|
reason: &str,
|
||||||
|
) -> (String, String) {
|
||||||
|
let number = extract_story_number(item_id).unwrap_or(item_id);
|
||||||
|
let name = story_name.unwrap_or(item_id);
|
||||||
|
|
||||||
|
let plain = format!("\u{1f6ab} #{number} {name} \u{2014} BLOCKED: {reason}");
|
||||||
|
let html = format!(
|
||||||
|
"\u{1f6ab} <strong>#{number}</strong> <em>{name}</em> \u{2014} BLOCKED: {reason}"
|
||||||
|
);
|
||||||
|
(plain, html)
|
||||||
|
}
|
||||||
|
|
||||||
/// Minimum time between rate-limit notifications for the same agent.
|
/// Minimum time between rate-limit notifications for the same agent.
|
||||||
const RATE_LIMIT_DEBOUNCE: Duration = Duration::from_secs(60);
|
const RATE_LIMIT_DEBOUNCE: Duration = Duration::from_secs(60);
|
||||||
|
|
||||||
@@ -249,6 +267,27 @@ pub fn spawn_notification_listener(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
Ok(WatcherEvent::StoryBlocked {
|
||||||
|
ref story_id,
|
||||||
|
ref reason,
|
||||||
|
}) => {
|
||||||
|
let story_name = find_story_name_any_stage(&project_root, story_id);
|
||||||
|
let (plain, html) = format_blocked_notification(
|
||||||
|
story_id,
|
||||||
|
story_name.as_deref(),
|
||||||
|
reason,
|
||||||
|
);
|
||||||
|
|
||||||
|
slog!("[bot] Sending blocked notification: {plain}");
|
||||||
|
|
||||||
|
for room_id in &get_room_ids() {
|
||||||
|
if let Err(e) = transport.send_message(room_id, &plain, &html).await {
|
||||||
|
slog!(
|
||||||
|
"[bot] Failed to send blocked notification to {room_id}: {e}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
Ok(_) => {} // Ignore non-work-item events
|
Ok(_) => {} // Ignore non-work-item events
|
||||||
Err(broadcast::error::RecvError::Lagged(n)) => {
|
Err(broadcast::error::RecvError::Lagged(n)) => {
|
||||||
slog!(
|
slog!(
|
||||||
@@ -622,6 +661,104 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ── format_blocked_notification ─────────────────────────────────────────
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn format_blocked_notification_with_story_name() {
|
||||||
|
let (plain, html) = format_blocked_notification(
|
||||||
|
"425_story_blocking_reason",
|
||||||
|
Some("Blocking Reason Story"),
|
||||||
|
"Retry limit exceeded (3/3) at coder stage",
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
plain,
|
||||||
|
"\u{1f6ab} #425 Blocking Reason Story \u{2014} BLOCKED: Retry limit exceeded (3/3) at coder stage"
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
html,
|
||||||
|
"\u{1f6ab} <strong>#425</strong> <em>Blocking Reason Story</em> \u{2014} BLOCKED: Retry limit exceeded (3/3) at coder stage"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn format_blocked_notification_falls_back_to_item_id() {
|
||||||
|
let (plain, _html) =
|
||||||
|
format_blocked_notification("42_story_thing", None, "empty diff");
|
||||||
|
assert_eq!(
|
||||||
|
plain,
|
||||||
|
"\u{1f6ab} #42 42_story_thing \u{2014} BLOCKED: empty diff"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
// ── spawn_notification_listener: StoryBlocked ───────────────────────────
|
||||||
|
|
||||||
|
/// AC1: when a StoryBlocked event arrives, send_message is called with a
|
||||||
|
/// notification that includes the story number, name, and reason.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn story_blocked_sends_notification_with_reason() {
|
||||||
|
let tmp = tempfile::tempdir().unwrap();
|
||||||
|
let stage_dir = tmp.path().join(".storkit").join("work").join("2_current");
|
||||||
|
std::fs::create_dir_all(&stage_dir).unwrap();
|
||||||
|
std::fs::write(
|
||||||
|
stage_dir.join("425_story_blocking_test.md"),
|
||||||
|
"---\nname: Blocking Test Story\n---\n",
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
let (watcher_tx, watcher_rx) = broadcast::channel::<WatcherEvent>(16);
|
||||||
|
let (transport, calls) = MockTransport::new();
|
||||||
|
|
||||||
|
spawn_notification_listener(
|
||||||
|
transport,
|
||||||
|
|| vec!["!room123:example.org".to_string()],
|
||||||
|
watcher_rx,
|
||||||
|
tmp.path().to_path_buf(),
|
||||||
|
);
|
||||||
|
|
||||||
|
watcher_tx.send(WatcherEvent::StoryBlocked {
|
||||||
|
story_id: "425_story_blocking_test".to_string(),
|
||||||
|
reason: "Retry limit exceeded (3/3) at coder stage".to_string(),
|
||||||
|
}).unwrap();
|
||||||
|
|
||||||
|
tokio::time::sleep(std::time::Duration::from_millis(100)).await;
|
||||||
|
|
||||||
|
let calls = calls.lock().unwrap();
|
||||||
|
assert_eq!(calls.len(), 1, "Expected exactly one notification");
|
||||||
|
let (room_id, plain, html) = &calls[0];
|
||||||
|
assert_eq!(room_id, "!room123:example.org");
|
||||||
|
assert!(plain.contains("425"), "plain should contain story number");
|
||||||
|
assert!(plain.contains("Blocking Test Story"), "plain should contain story name");
|
||||||
|
assert!(plain.contains("BLOCKED"), "plain should contain BLOCKED label");
|
||||||
|
assert!(plain.contains("Retry limit exceeded"), "plain should contain the reason");
|
||||||
|
assert!(html.contains("BLOCKED"), "html should contain BLOCKED label");
|
||||||
|
}
|
||||||
|
|
||||||
|
/// StoryBlocked with no room registered should not panic.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn story_blocked_with_no_rooms_is_silent() {
|
||||||
|
let tmp = tempfile::tempdir().unwrap();
|
||||||
|
|
||||||
|
let (watcher_tx, watcher_rx) = broadcast::channel::<WatcherEvent>(16);
|
||||||
|
let (transport, calls) = MockTransport::new();
|
||||||
|
|
||||||
|
spawn_notification_listener(
|
||||||
|
transport,
|
||||||
|
Vec::new,
|
||||||
|
watcher_rx,
|
||||||
|
tmp.path().to_path_buf(),
|
||||||
|
);
|
||||||
|
|
||||||
|
watcher_tx.send(WatcherEvent::StoryBlocked {
|
||||||
|
story_id: "42_story_no_rooms".to_string(),
|
||||||
|
reason: "empty diff".to_string(),
|
||||||
|
}).unwrap();
|
||||||
|
|
||||||
|
tokio::time::sleep(std::time::Duration::from_millis(100)).await;
|
||||||
|
|
||||||
|
let calls = calls.lock().unwrap();
|
||||||
|
assert_eq!(calls.len(), 0, "No rooms means no notifications");
|
||||||
|
}
|
||||||
|
|
||||||
// ── format_rate_limit_notification ─────────────────────────────────────
|
// ── format_rate_limit_notification ─────────────────────────────────────
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
|||||||
@@ -158,10 +158,11 @@ impl From<WatcherEvent> for Option<WsResponse> {
|
|||||||
}),
|
}),
|
||||||
WatcherEvent::ConfigChanged => Some(WsResponse::AgentConfigChanged),
|
WatcherEvent::ConfigChanged => Some(WsResponse::AgentConfigChanged),
|
||||||
WatcherEvent::AgentStateChanged => Some(WsResponse::AgentStateChanged),
|
WatcherEvent::AgentStateChanged => Some(WsResponse::AgentStateChanged),
|
||||||
// MergeFailure and RateLimitWarning are handled by the chat notification
|
// MergeFailure, RateLimitWarning, and StoryBlocked are handled by the
|
||||||
// listener only; no WebSocket message is needed for the frontend.
|
// chat notification listener only; no WebSocket message is needed for the frontend.
|
||||||
WatcherEvent::MergeFailure { .. } => None,
|
WatcherEvent::MergeFailure { .. } => None,
|
||||||
WatcherEvent::RateLimitWarning { .. } => None,
|
WatcherEvent::RateLimitWarning { .. } => None,
|
||||||
|
WatcherEvent::StoryBlocked { .. } => None,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -67,6 +67,14 @@ pub enum WatcherEvent {
|
|||||||
/// Name of the agent that hit the rate limit.
|
/// Name of the agent that hit the rate limit.
|
||||||
agent_name: String,
|
agent_name: String,
|
||||||
},
|
},
|
||||||
|
/// A story has been blocked (e.g. retry limit exceeded, empty diff).
|
||||||
|
/// Triggers a warning notification to configured chat rooms.
|
||||||
|
StoryBlocked {
|
||||||
|
/// Work item ID (e.g. `"42_story_my_feature"`).
|
||||||
|
story_id: String,
|
||||||
|
/// Human-readable reason the story was blocked.
|
||||||
|
reason: String,
|
||||||
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Return `true` if `path` is the root-level `.storkit/project.toml`, i.e.
|
/// Return `true` if `path` is the root-level `.storkit/project.toml`, i.e.
|
||||||
|
|||||||
Reference in New Issue
Block a user