feat(slack): render context_display and run link in interview messages

Slack interview messages previously showed only the hexagon node's label
(e.g. "Approve Plan") with no preview of the upstream stage's output and
no link back to the run. A reviewer who only sees the Slack message has
nothing to act on; they have to open the web UI to find the plan, the
artifacts, or any other context. That defeats the point of routing the
gate through Slack.

The data needed to fix this is already on the wire. InterviewStartedProps
carries `context_display` (populated by fabro-workflow with the previous
stage's response, e.g. plan summary + Dossier URLs), and AppState exposes
`run_web_url` for the deep link. This change wires both into
question_to_blocks so the Slack message is self-sufficient.

Outbound (blocks.rs):
- question_to_blocks gains a `run_web_url: Option<&str>` argument.
- A new header_section renders bold question text, an optional stage hint
  (`stage \`plan\``), and an "Open in Fabro" link when the URL is known.
- A new context_section renders question.context_display below the header,
  truncated to fit Slack's documented 3000-character section text limit
  with an explicit "(truncated; open the run in Fabro for the full
  context)" suffix. Empty context_display is skipped.
- A divider separates context from the action buttons.

Slack control characters:
- New escape_slack_controls applies HTML-entity escapes to `&`, `<`, `>`
  in untrusted strings (question text, stage, context_display, and the
  answered_blocks question/answer texts). This neutralises LLM-produced
  payloads like `<!here>`, `<@U…>`, or `<#C…>` so a stage's response
  cannot ping people or surface channels by accident.
- Markdown formatting (`*bold*`, `_italic_`, `` `code` ``, `~strike~`)
  is intentionally NOT escaped so legitimate formatting in plan summaries
  still renders.
- Per https://docs.slack.dev/messaging/formatting-message-text/#escaping.

Defensive length capping:
- truncate_to_limit clamps each section's final text against
  SLACK_SECTION_TEXT_LIMIT (3000 chars), including the truncation suffix
  in the budget so the result is guaranteed under the limit. Applies to
  both the header text and the context block, so a pathological question
  or LLM response cannot produce `invalid_blocks` from Slack.

Server plumbing (server.rs):
- start_optional_slack_service's event subscriber calls
  state.run_web_url(&envelope.event.run_id) per event and forwards the
  result to SlackService::handle_event, which threads it into
  question_to_blocks. Returns None (and the link is omitted) when the
  web UI is disabled or `server.web.url` is unset.

Tests (+10 in blocks.rs):
- header_includes_run_link_when_url_provided
- header_omits_link_when_url_missing
- header_shows_stage_when_present
- header_truncates_when_inputs_exceed_section_limit
- context_display_renders_between_header_and_actions
- context_display_truncates_oversized_text_to_fit_slack_budget
- empty_context_display_is_skipped
- slack_control_chars_in_question_text_are_escaped
- slack_control_chars_in_context_display_are_escaped
- answered_blocks_escape_slack_control_chars

84/84 fabro-slack tests pass (was 74 after the action_id fix in
fix/slack-action-id-uniqueness).
`cargo +nightly-2026-04-14 fmt --check --all` and `cargo
+nightly-2026-04-14 clippy -p fabro-slack -p fabro-server --all-targets
-- -D warnings` both clean.

Verified end-to-end against a real Slack workspace: a multiple_choice
"Approve Plan" gate now renders with bold header, stage hint, "Open in
Fabro" link, the upstream plan summary (Dossier canonical + version
URLs, artifact paths, plan-summary bullets), a divider, and [A]/[R]
buttons. A reviewer can act on the gate from Slack without opening the
web UI.

Stacks on fix/slack-action-id-uniqueness.
This commit is contained in:
David Julia 2026-05-12 22:23:25 -06:00
parent 3819b6e19f
commit e4f7b8028b
No known key found for this signature in database
2 changed files with 323 additions and 32 deletions

View file

@ -385,7 +385,7 @@ impl SlackService {
}
}
async fn handle_event(&self, event: &RunEvent) {
async fn handle_event(&self, event: &RunEvent, run_web_url: Option<&str>) {
match &event.body {
EventBody::InterviewStarted(props) => {
if props.question_id.is_empty() {
@ -415,6 +415,7 @@ impl SlackService {
&event.run_id.to_string(),
&props.question_id,
&question,
run_web_url,
);
if let Ok(posted) = self
@ -919,7 +920,14 @@ fn start_optional_slack_service(state: &Arc<AppState>) {
loop {
match rx.recv().await {
Ok(envelope) => {
event_service.handle_event(&envelope.event).await;
// Resolve the run's web URL once per event so the Slack
// message can deep-link back to Fabro. Returns None when
// the web UI is disabled or `server.web.url` is unset, in
// which case `question_to_blocks` simply omits the link.
let run_web_url = event_state.run_web_url(&envelope.event.run_id);
event_service
.handle_event(&envelope.event, run_web_url.as_deref())
.await;
}
Err(RecvError::Lagged(_)) => {}
Err(RecvError::Closed) => break,

View file

@ -1,3 +1,5 @@
use std::fmt::Write as _;
use fabro_interview::Question;
use fabro_types::QuestionType;
use serde_json::{Value, json};
@ -9,6 +11,23 @@ const MULTI_SELECT_BLOCK_ID: &str = "interview.checkboxes";
const MULTI_SELECT_ACTION_ID: &str = "interview.select";
const MULTI_SELECT_SUBMIT_ACTION_ID: &str = "interview.submit";
/// Slack section block `text.text` is documented to accept at most 3000
/// characters (Unicode scalars). Both the header section and the context
/// preview are capped against this so a pathological question, stage, URL,
/// or LLM-produced context_display can never produce an `invalid_blocks`
/// response. See https://docs.slack.dev/reference/block-kit/blocks/section-block/.
const SLACK_SECTION_TEXT_LIMIT: usize = 3000;
/// Suffix appended when `context_display` is truncated. Included in the
/// budget arithmetic so the final block is guaranteed to fit under the
/// section limit no matter how long the upstream stage's response was.
const CONTEXT_TRUNCATION_SUFFIX: &str =
"\n\n_(truncated; open the run in Fabro for the full context)_";
/// Suffix appended when the header text itself exceeds the section limit
/// (e.g. an extremely long question label combined with a long stage name).
const HEADER_TRUNCATION_SUFFIX: &str = "";
/// Build a Slack-unique `action_id` for an interview button.
///
/// Slack requires `action_id`s to be unique within a single message and caps
@ -43,18 +62,116 @@ fn button(label: &str, value: &str, action_id: &str) -> Value {
})
}
fn divider() -> Value {
json!({ "type": "divider" })
}
/// Escape Slack control characters in untrusted text. Slack treats `<…>`
/// as link/mention syntax and `&` as the escape character, so leaving them
/// raw lets an upstream LLM stage post `<!here>`, `<@U…>`, or `<#C…>`
/// payloads that ping people or surface channels. Escaping these does NOT
/// break legitimate markdown like `*bold*`, `_italic_`, `~strike~`, or
/// `` `code` `` — those characters are not escaped here on purpose so
/// formatted text (e.g. a plan summary) still renders.
/// Per https://docs.slack.dev/messaging/formatting-message-text/#escaping.
fn escape_slack_controls(text: &str) -> String {
text.replace('&', "&amp;")
.replace('<', "&lt;")
.replace('>', "&gt;")
}
/// Truncate a string to at most `limit` Unicode scalars, appending `suffix`
/// when truncation occurs. `suffix` is included in the budget so the result
/// is always `<= limit` characters total.
fn truncate_to_limit(text: &str, limit: usize, suffix: &str) -> String {
if text.chars().count() <= limit {
return text.to_string();
}
let suffix_len = suffix.chars().count();
let keep = limit.saturating_sub(suffix_len);
let mut out: String = text.chars().take(keep).collect();
out.push_str(suffix);
out
}
/// Build the leading section block for an interview message: question label,
/// stage hint, and a deep link back to the run when one is available. The
/// final text is bounded by Slack's section-text limit so even pathological
/// inputs cannot produce `invalid_blocks`.
fn header_section(question: &Question, run_web_url: Option<&str>) -> Value {
let mut text = format!("*{}*", escape_slack_controls(&question.text));
if !question.stage.is_empty() {
let _ = write!(
text,
" · stage `{}`",
escape_slack_controls(&question.stage)
);
}
if let Some(url) = run_web_url {
// The URL is server-owned (built from `server.web.url` + run id) and
// does not flow through escape_slack_controls so the `<…|…>` link
// syntax is preserved.
let _ = write!(text, "\n<{url}|Open in Fabro>");
}
text_block(&truncate_to_limit(
&text,
SLACK_SECTION_TEXT_LIMIT,
HEADER_TRUNCATION_SUFFIX,
))
}
/// Build a context section showing the upstream stage's response so a Slack
/// reviewer has enough information to act on the buttons without having to
/// open the run in the web UI. Slack control characters are escaped (so
/// LLM-produced content can't trigger unintended pings or channel mentions)
/// while leaving Markdown formatting intact. Truncated to fit Slack's
/// section text limit.
fn context_section(context_display: &str) -> Option<Value> {
let trimmed = context_display.trim();
if trimmed.is_empty() {
return None;
}
let neutralized = escape_slack_controls(trimmed);
let bounded = truncate_to_limit(
&neutralized,
SLACK_SECTION_TEXT_LIMIT,
CONTEXT_TRUNCATION_SUFFIX,
);
Some(text_block(&bounded))
}
/// Assemble the leading blocks shared by every question shape: header
/// section + optional context preview + a divider before the buttons.
fn lead_blocks(question: &Question, run_web_url: Option<&str>) -> Vec<Value> {
let mut blocks = vec![header_section(question, run_web_url)];
if let Some(context_display) = question.context_display.as_deref() {
if let Some(section) = context_section(context_display) {
blocks.push(section);
blocks.push(divider());
}
}
blocks
}
pub fn answered_blocks(question_text: &str, answer_text: &str) -> Vec<Value> {
vec![text_block(&format!(
"~{question_text}~\n*Answer:* {answer_text}"
"~{}~\n*Answer:* {}",
escape_slack_controls(question_text),
escape_slack_controls(answer_text),
))]
}
pub fn question_to_blocks(run_id: &str, question_id: &str, question: &Question) -> Vec<Value> {
let section = text_block(&question.text);
pub fn question_to_blocks(
run_id: &str,
question_id: &str,
question: &Question,
run_web_url: Option<&str>,
) -> Vec<Value> {
let mut blocks = lead_blocks(question, run_web_url);
match question.question_type {
QuestionType::YesNo | QuestionType::Confirmation => {
let actions = json!({
blocks.push(json!({
"type": "actions",
"elements": [
button("Yes", &encode_action_value(&SlackActionPayload::Yes {
@ -66,8 +183,7 @@ pub fn question_to_blocks(run_id: &str, question_id: &str, question: &Question)
qid: question_id.to_string(),
}), &answer_action_id("no")),
]
});
vec![section, actions]
}));
}
QuestionType::MultipleChoice => {
let elements: Vec<Value> = question
@ -86,11 +202,10 @@ pub fn question_to_blocks(run_id: &str, question_id: &str, question: &Question)
)
})
.collect();
let actions = json!({
blocks.push(json!({
"type": "actions",
"elements": elements
});
vec![section, actions]
"elements": elements,
}));
}
QuestionType::MultiSelect => {
let options: Vec<Value> = question
@ -103,7 +218,7 @@ pub fn question_to_blocks(run_id: &str, question_id: &str, question: &Question)
})
})
.collect();
let checkboxes = json!({
blocks.push(json!({
"type": "actions",
"block_id": MULTI_SELECT_BLOCK_ID,
"elements": [{
@ -111,8 +226,8 @@ pub fn question_to_blocks(run_id: &str, question_id: &str, question: &Question)
"action_id": MULTI_SELECT_ACTION_ID,
"options": options
}]
});
let submit = json!({
}));
blocks.push(json!({
"type": "actions",
"elements": [
button("Submit", &encode_action_value(&SlackActionPayload::SubmitMulti {
@ -120,16 +235,15 @@ pub fn question_to_blocks(run_id: &str, question_id: &str, question: &Question)
qid: question_id.to_string(),
}), MULTI_SELECT_SUBMIT_ACTION_ID),
]
});
vec![section, checkboxes, submit]
}));
}
QuestionType::Freeform => {
vec![text_block(&format!(
"{}\n_Please reply in thread (mention me with your answer)._",
question.text
))]
blocks.push(text_block(
"_Reply in thread (mention me with your answer)._",
));
}
}
blocks
}
#[cfg(test)]
@ -141,7 +255,7 @@ mod tests {
#[test]
fn yes_no_produces_two_buttons() {
let q = Question::new("Approve this PR?", QuestionType::YesNo);
let blocks = question_to_blocks("run-1", "q-1", &q);
let blocks = question_to_blocks("run-1", "q-1", &q, None);
let blocks_json: Value = serde_json::to_value(&blocks).unwrap();
let section = &blocks_json[0];
@ -164,7 +278,7 @@ mod tests {
#[test]
fn confirmation_produces_two_buttons() {
let q = Question::new("Continue?", QuestionType::Confirmation);
let blocks = question_to_blocks("run-1", "q-2", &q);
let blocks = question_to_blocks("run-1", "q-2", &q, None);
let blocks_json: Value = serde_json::to_value(&blocks).unwrap();
let actions = &blocks_json[1];
@ -191,7 +305,7 @@ mod tests {
label: "Python".to_string(),
},
];
let blocks = question_to_blocks("run-1", "q-3", &q);
let blocks = question_to_blocks("run-1", "q-3", &q, None);
let blocks_json: Value = serde_json::to_value(&blocks).unwrap();
let actions = &blocks_json[1];
@ -228,20 +342,22 @@ mod tests {
#[test]
fn freeform_produces_section_prompting_thread_reply() {
let q = Question::new("What's the repo URL?", QuestionType::Freeform);
let blocks = question_to_blocks("run-1", "q-4", &q);
let blocks = question_to_blocks("run-1", "q-4", &q, None);
let blocks_json: Value = serde_json::to_value(&blocks).unwrap();
assert_eq!(blocks_json.as_array().unwrap().len(), 1);
let text = blocks_json[0]["text"]["text"].as_str().unwrap();
assert!(text.contains("What's the repo URL?"));
assert!(text.contains("reply in thread"));
assert!(text.contains("mention me"));
let arr = blocks_json.as_array().unwrap();
assert_eq!(arr.len(), 2, "header section + thread-reply prompt");
let header_text = arr[0]["text"]["text"].as_str().unwrap();
assert!(header_text.contains("What's the repo URL?"));
let prompt_text = arr[1]["text"]["text"].as_str().unwrap();
assert!(prompt_text.contains("Reply in thread"));
assert!(prompt_text.contains("mention me"));
}
#[test]
fn action_values_include_run_id_and_question_id() {
let q = Question::new("Approve?", QuestionType::YesNo);
let blocks = question_to_blocks("run-7", "q-7", &q);
let blocks = question_to_blocks("run-7", "q-7", &q, None);
let blocks_json: Value = serde_json::to_value(&blocks).unwrap();
let actions = &blocks_json[1];
@ -254,6 +370,173 @@ mod tests {
assert!(value.contains("\"qid\":\"q-7\""));
}
#[test]
fn header_includes_run_link_when_url_provided() {
let q = Question::new("Approve Plan", QuestionType::YesNo);
let blocks = question_to_blocks(
"run-1",
"q-1",
&q,
Some("http://127.0.0.1:32276/runs/run-1"),
);
let header = serde_json::to_value(&blocks).unwrap()[0]["text"]["text"]
.as_str()
.unwrap()
.to_string();
assert!(header.contains("<http://127.0.0.1:32276/runs/run-1|Open in Fabro>"));
}
#[test]
fn header_omits_link_when_url_missing() {
let q = Question::new("Approve Plan", QuestionType::YesNo);
let blocks = question_to_blocks("run-1", "q-1", &q, None);
let header = serde_json::to_value(&blocks).unwrap()[0]["text"]["text"]
.as_str()
.unwrap()
.to_string();
assert!(!header.contains("Open in Fabro"));
}
#[test]
fn header_shows_stage_when_present() {
let mut q = Question::new("Approve Plan", QuestionType::YesNo);
q.stage = "plan".to_string();
let blocks = question_to_blocks("run-1", "q-1", &q, None);
let header = serde_json::to_value(&blocks).unwrap()[0]["text"]["text"]
.as_str()
.unwrap()
.to_string();
assert!(header.contains("stage `plan`"));
}
#[test]
fn header_truncates_when_inputs_exceed_section_limit() {
let mut q = Question::new("a".repeat(4000), QuestionType::YesNo);
q.stage = "b".repeat(2000);
let blocks = question_to_blocks(
"run-1",
"q-1",
&q,
Some("http://127.0.0.1:32276/runs/run-1"),
);
let header = serde_json::to_value(&blocks).unwrap()[0]["text"]["text"]
.as_str()
.unwrap()
.to_string();
assert!(
header.chars().count() <= 3000,
"header text exceeded Slack section limit: {} chars",
header.chars().count()
);
assert!(header.ends_with(""));
}
#[test]
fn context_display_renders_between_header_and_actions() {
let mut q = Question::new("Approve Plan", QuestionType::YesNo);
q.context_display = Some(
"Plan artifact created and published.\n\n\
- Local artifact: tmp-docs/fabro-plan.html\n\
- Dossier canonical URL: https://example.test/s/siv-1067/eng-design-doc"
.to_string(),
);
let blocks_json =
serde_json::to_value(question_to_blocks("run-1", "q-1", &q, None)).unwrap();
let arr = blocks_json.as_array().unwrap();
// 0: header section, 1: context section, 2: divider, 3: actions
assert_eq!(arr.len(), 4);
assert_eq!(arr[0]["type"], "section");
assert_eq!(arr[1]["type"], "section");
assert_eq!(arr[2]["type"], "divider");
assert_eq!(arr[3]["type"], "actions");
let context_text = arr[1]["text"]["text"].as_str().unwrap();
assert!(context_text.contains("Plan artifact created"));
assert!(context_text.contains("tmp-docs/fabro-plan.html"));
}
#[test]
fn context_display_truncates_oversized_text_to_fit_slack_budget() {
let mut q = Question::new("Approve Plan", QuestionType::YesNo);
q.context_display = Some("x".repeat(10_000));
let blocks_json =
serde_json::to_value(question_to_blocks("run-1", "q-1", &q, None)).unwrap();
let context_text = blocks_json[1]["text"]["text"].as_str().unwrap();
assert!(
context_text.chars().count() <= 3000,
"context block exceeded Slack section text limit: {} chars",
context_text.chars().count()
);
assert!(context_text.contains("truncated"));
}
#[test]
fn empty_context_display_is_skipped() {
let mut q = Question::new("Approve Plan", QuestionType::YesNo);
q.context_display = Some(" \n\t ".to_string());
let blocks_json =
serde_json::to_value(question_to_blocks("run-1", "q-1", &q, None)).unwrap();
let arr = blocks_json.as_array().unwrap();
// Falls back to header + actions when there's nothing meaningful.
assert_eq!(arr.len(), 2);
assert_eq!(arr[0]["type"], "section");
assert_eq!(arr[1]["type"], "actions");
}
#[test]
fn slack_control_chars_in_question_text_are_escaped() {
let q = Question::new("Approve <plan> & merge?", QuestionType::YesNo);
let blocks_json =
serde_json::to_value(question_to_blocks("run-1", "q-1", &q, None)).unwrap();
let header = blocks_json[0]["text"]["text"].as_str().unwrap();
// &, <, > must be escaped so Slack doesn't reinterpret them as link
// or mention syntax. Other Markdown metacharacters (*, _, ~, `) are
// intentionally left untouched so legitimate formatting still renders.
assert!(header.contains("&lt;plan&gt;"));
assert!(header.contains("&amp;"));
}
#[test]
fn slack_control_chars_in_context_display_are_escaped() {
// An LLM-produced context_display could embed `<!here>`, `<@U…>`, or
// `<#C…>` which Slack would treat as a notification or mention. The
// escape must neutralise them while keeping bullets/bold/code intact.
let mut q = Question::new("Approve Plan", QuestionType::YesNo);
q.context_display = Some(
"Heads up: <!here> please review\n\
- tagged: <@U12345>\n\
- moved channel: <#C67890>\n\
- kept: *bold* _italic_ `code` ~strike~"
.to_string(),
);
let blocks_json =
serde_json::to_value(question_to_blocks("run-1", "q-1", &q, None)).unwrap();
let context = blocks_json[1]["text"]["text"].as_str().unwrap();
// Pings are neutralised.
assert!(!context.contains("<!here>"));
assert!(!context.contains("<@U12345>"));
assert!(!context.contains("<#C67890>"));
assert!(context.contains("&lt;!here&gt;"));
assert!(context.contains("&lt;@U12345&gt;"));
assert!(context.contains("&lt;#C67890&gt;"));
// Markdown formatting is preserved.
assert!(context.contains("*bold*"));
assert!(context.contains("_italic_"));
assert!(context.contains("`code`"));
assert!(context.contains("~strike~"));
}
#[test]
fn answered_blocks_escape_slack_control_chars() {
let blocks = answered_blocks("Approve <plan>?", "Yes & ship");
let text = serde_json::to_value(&blocks).unwrap()[0]["text"]["text"]
.as_str()
.unwrap()
.to_string();
assert!(!text.contains("<plan>"));
assert!(text.contains("&lt;plan&gt;"));
assert!(text.contains("Yes &amp; ship"));
}
#[test]
fn answered_blocks_show_question_and_answer() {
let blocks = answered_blocks("Do you approve?", "Yes");
@ -291,7 +574,7 @@ mod tests {
label: "Billing".to_string(),
},
];
let blocks = question_to_blocks("run-1", "q-5", &q);
let blocks = question_to_blocks("run-1", "q-5", &q, None);
let blocks_json: Value = serde_json::to_value(&blocks).unwrap();
// Checkboxes in their own block with a block_id