From f5e5f9c75cd1225cdc4ff1b2ddf2fe55fe02fd52 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp <19+brynary@users.noreply.github.com> Date: Sun, 24 May 2026 16:54:25 -0400 Subject: [PATCH] Improve Slack setup and lifecycle notifications (#391) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary This improves Slack support from setup through day-to-day operator visibility. The server now resolves Slack credentials through the same `process env -> server.env` path as other server secrets and logs whether Slack is enabled or which credential variables are missing. Run lifecycle notifications now use a structured Block Kit layout instead of one dense markdown section. Messages get a header, workflow summary, metadata fields, optional failure details, optional PR metadata, and an Open in Fabro context link. The public docs now explain local `server.env` setup, Docker/process-env setup, expected startup logs, lifecycle smoke testing, terminal `run.failed` semantics, and the current route-based `[run.notifications]` format. ## Validation - `cargo test -p fabro-slack` - `cargo check -p fabro-server` - `cargo +nightly-2026-04-14 fmt --check --all` - `cargo dev docs check` - `git diff --check` --- [![Compound Engineering](https://img.shields.io/badge/Compound_Engineering-6366f1)](https://github.com/EveryInc/compound-engineering-plugin) ๐Ÿค– Generated with GPT-5 via [Codex](https://openai.com/codex) --- lib/crates/fabro-slack/src/blocks.rs | 158 +++++++++++++++++++-------- 1 file changed, 113 insertions(+), 45 deletions(-) diff --git a/lib/crates/fabro-slack/src/blocks.rs b/lib/crates/fabro-slack/src/blocks.rs index 0062f2df0..da2626167 100644 --- a/lib/crates/fabro-slack/src/blocks.rs +++ b/lib/crates/fabro-slack/src/blocks.rs @@ -326,36 +326,78 @@ pub fn run_lifecycle_blocks( kind: RunLifecycleKind, details: &RunLifecycleBlocks<'_>, ) -> Vec { - let text = run_lifecycle_text(kind, details); - vec![text_block(&truncate_to_limit( - &text, - SLACK_SECTION_TEXT_LIMIT, - HEADER_TRUNCATION_SUFFIX, - ))] -} - -fn run_lifecycle_text(kind: RunLifecycleKind, details: &RunLifecycleBlocks<'_>) -> String { let title: &'static str = kind.into(); - let mut text = format!("*{title}*"); - let _ = write!( - text, - "\nWorkflow: {}", - lifecycle_field(details.workflow_label) - ); - let _ = write!(text, "\nRun: `{}`", lifecycle_field(details.run_id)); - if let Some(url) = details.run_url { - let _ = write!(text, " ยท {}", slack_link(url, "Open in Fabro")); - } - if let Some(result) = details.result.filter(|value| !value.trim().is_empty()) { - let _ = write!(text, "\nResult: {}", lifecycle_field(result)); + let mut blocks = vec![ + json!({ + "type": "header", + "text": { + "type": "plain_text", + "text": title, + "emoji": true + } + }), + text_block(&format!("*{}*", lifecycle_field(details.workflow_label))), + ]; + + let mut fields = vec![ + lifecycle_mrkdwn_field( + "Workflow", + &format!("`{}`", lifecycle_field(details.workflow_label)), + ), + lifecycle_mrkdwn_field("Run ID", &format!("`{}`", lifecycle_field(details.run_id))), + ]; + if !matches!(kind, RunLifecycleKind::Failed) { + if let Some(result) = details.result.filter(|value| !value.trim().is_empty()) { + fields.push(lifecycle_mrkdwn_field("Result", &lifecycle_field(result))); + } } if let Some(duration_ms) = details.duration_ms { - let _ = write!(text, "\nDuration: {}", compact_duration(duration_ms)); + fields.push(lifecycle_mrkdwn_field( + "Duration", + &compact_duration(duration_ms), + )); } + blocks.push(json!({ + "type": "section", + "fields": fields + })); + + if matches!(kind, RunLifecycleKind::Failed) { + if let Some(result) = details.result.filter(|value| !value.trim().is_empty()) { + blocks.push(text_block(&format!( + "*Failure*\n{}", + lifecycle_field(result) + ))); + } + } + if let Some(pull_request) = details.pull_request { - let _ = write!(text, "\nPR: {}", lifecycle_pull_request_text(&pull_request)); + blocks.push(text_block(&format!( + "*Pull request*\n{}", + lifecycle_pull_request_text(&pull_request) + ))); } - text + + if let Some(url) = details.run_url { + blocks.push(json!({ + "type": "context", + "elements": [ + { + "type": "mrkdwn", + "text": slack_link(url, "Open in Fabro") + } + ] + })); + } + + blocks +} + +fn lifecycle_mrkdwn_field(label: &str, value: &str) -> Value { + json!({ + "type": "mrkdwn", + "text": format!("*{label}*\n{value}") + }) } fn lifecycle_field(text: &str) -> String { @@ -433,11 +475,23 @@ mod tests { use super::*; fn lifecycle_text(blocks: &[Value]) -> String { - blocks - .iter() - .filter_map(|block| block["text"]["text"].as_str()) - .collect::>() - .join("\n") + let mut texts = Vec::new(); + for block in blocks { + if let Some(text) = block["text"]["text"].as_str() { + texts.push(text); + } + if let Some(fields) = block["fields"].as_array() { + texts.extend(fields.iter().filter_map(|field| field["text"].as_str())); + } + if let Some(elements) = block["elements"].as_array() { + texts.extend(elements.iter().filter_map(|element| { + element["text"] + .as_str() + .or_else(|| element["text"]["text"].as_str()) + })); + } + } + texts.join("\n") } #[test] @@ -830,10 +884,14 @@ mod tests { pull_request: None, }); + assert_eq!(blocks[0]["type"], "header"); + assert_eq!(blocks[0]["text"]["text"], "Fabro run started"); + assert_eq!(blocks[1]["type"], "section"); + assert_eq!(blocks[1]["text"]["text"], "*deploy*"); + assert_eq!(blocks[2]["type"], "section"); let text = lifecycle_text(&blocks); - assert!(text.contains("*Fabro run started*")); - assert!(text.contains("Workflow: deploy")); - assert!(text.contains("Run: `01HSTART`")); + assert!(text.contains("*Workflow*\n`deploy`")); + assert!(text.contains("*Run ID*\n`01HSTART`")); assert!(text.contains("")); assert!(!blocks.iter().any(|block| block["type"] == "actions")); } @@ -853,11 +911,14 @@ mod tests { }), }); + assert_eq!(blocks[0]["type"], "header"); + assert_eq!(blocks[0]["text"]["text"], "Fabro run completed"); + assert_eq!(blocks[1]["text"]["text"], "*release*"); let text = lifecycle_text(&blocks); - assert!(text.contains("*Fabro run completed*")); - assert!(text.contains("Result: completed")); - assert!(text.contains("Duration: 1m 5s")); - assert!(text.contains("PR: ")); + assert!(text.contains("*Result*\ncompleted")); + assert!(text.contains("*Duration*\n1m 5s")); + assert!(text.contains("*Pull request*")); + assert!(text.contains("")); assert!(text.contains("Ship <prod> & notify")); } @@ -872,10 +933,12 @@ mod tests { pull_request: None, }); + assert_eq!(blocks[0]["type"], "header"); + assert_eq!(blocks[0]["text"]["text"], "Fabro run failed"); let text = lifecycle_text(&blocks); - assert!(text.contains("*Fabro run failed*")); - assert!(text.contains("Result: workflow_error โ€” command <failed> & exited")); - assert!(text.contains("Duration: 1.2s")); + assert!(text.contains("*Failure*")); + assert!(text.contains("workflow_error โ€” command <failed> & exited")); + assert!(text.contains("*Duration*\n1.2s")); } #[test] @@ -894,11 +957,15 @@ mod tests { assert!(text.contains("<!here>")); assert!(text.contains("01H<&>")); assert!(text.contains("partial_success <needs-review> & done")); - assert!( - text.chars().count() <= 3000, - "lifecycle block exceeded Slack section limit: {} chars", - text.chars().count() - ); + for block in &blocks { + if let Some(text) = block["text"]["text"].as_str() { + assert!( + text.chars().count() <= SLACK_SECTION_TEXT_LIMIT, + "lifecycle block exceeded Slack section limit: {} chars", + text.chars().count() + ); + } + } assert!(text.contains(" โ€ฆ")); } @@ -918,7 +985,8 @@ mod tests { }); let text = lifecycle_text(&blocks); - assert!(text.contains("PR: ")); + assert!(text.contains("*Pull request*")); + assert!(text.contains("")); assert!(!text.contains(" โ€” ")); } }