From b99895e873fcc576db5660df45aabe6a503ff115 Mon Sep 17 00:00:00 2001 From: "fabro-releases[bot]" Date: Wed, 13 May 2026 10:06:39 +0000 Subject: [PATCH 1/3] Bump version to 0.232.0-nightly.0 --- Cargo.lock | 92 +++++++++++++++++++++++++++--------------------------- Cargo.toml | 2 +- 2 files changed, 47 insertions(+), 47 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 54c39c172..850319cd3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1580,7 +1580,7 @@ dependencies = [ [[package]] name = "fabro-acp" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "agent-client-protocol", "agent-client-protocol-tokio", @@ -1602,7 +1602,7 @@ dependencies = [ [[package]] name = "fabro-agent" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "anyhow", "async-trait", @@ -1641,7 +1641,7 @@ dependencies = [ [[package]] name = "fabro-api" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "chrono", "fabro-config", @@ -1662,7 +1662,7 @@ dependencies = [ [[package]] name = "fabro-auth" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "anyhow", "async-trait", @@ -1686,11 +1686,11 @@ dependencies = [ [[package]] name = "fabro-build-support" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" [[package]] name = "fabro-checkpoint" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "chrono", "fabro-config", @@ -1706,7 +1706,7 @@ dependencies = [ [[package]] name = "fabro-cli" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "anyhow", "assert_cmd", @@ -1807,7 +1807,7 @@ dependencies = [ [[package]] name = "fabro-client" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "anyhow", "bytes", @@ -1836,7 +1836,7 @@ dependencies = [ [[package]] name = "fabro-config" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "anyhow", "chrono", @@ -1864,7 +1864,7 @@ dependencies = [ [[package]] name = "fabro-core" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "async-trait", "fabro-types", @@ -1879,7 +1879,7 @@ dependencies = [ [[package]] name = "fabro-dev" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "anyhow", "assert_cmd", @@ -1898,7 +1898,7 @@ dependencies = [ [[package]] name = "fabro-devcontainer" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "fabro-http", "fabro-static", @@ -1915,7 +1915,7 @@ dependencies = [ [[package]] name = "fabro-dump" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "anyhow", "bytes", @@ -1929,7 +1929,7 @@ dependencies = [ [[package]] name = "fabro-github" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "anyhow", "base64", @@ -1951,7 +1951,7 @@ dependencies = [ [[package]] name = "fabro-graphviz" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "anyhow", "fabro-types", @@ -1965,7 +1965,7 @@ dependencies = [ [[package]] name = "fabro-hooks" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "async-trait", "fabro-agent", @@ -1989,7 +1989,7 @@ dependencies = [ [[package]] name = "fabro-http" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "fabro-static", "http", @@ -1999,7 +1999,7 @@ dependencies = [ [[package]] name = "fabro-install" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "anyhow", "base64", @@ -2014,7 +2014,7 @@ dependencies = [ [[package]] name = "fabro-interview" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "async-trait", "dialoguer", @@ -2029,7 +2029,7 @@ dependencies = [ [[package]] name = "fabro-llm" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "anyhow", "async-trait", @@ -2061,7 +2061,7 @@ dependencies = [ [[package]] name = "fabro-macros" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "clap", "fabro-options-metadata", @@ -2072,7 +2072,7 @@ dependencies = [ [[package]] name = "fabro-manifest" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "anyhow", "fabro-api", @@ -2090,7 +2090,7 @@ dependencies = [ [[package]] name = "fabro-mcp" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "anyhow", "fabro-config", @@ -2106,7 +2106,7 @@ dependencies = [ [[package]] name = "fabro-mcp-server" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "anyhow", "chrono", @@ -2128,7 +2128,7 @@ dependencies = [ [[package]] name = "fabro-model" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "fabro-static", "insta", @@ -2142,7 +2142,7 @@ dependencies = [ [[package]] name = "fabro-oauth" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "anyhow", "axum", @@ -2164,7 +2164,7 @@ dependencies = [ [[package]] name = "fabro-options-metadata" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "serde", "serde_json", @@ -2172,7 +2172,7 @@ dependencies = [ [[package]] name = "fabro-proc" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "cc", "libc", @@ -2181,7 +2181,7 @@ dependencies = [ [[package]] name = "fabro-redact" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "aho-corasick", "ref-cast", @@ -2197,7 +2197,7 @@ dependencies = [ [[package]] name = "fabro-sandbox" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "anyhow", "async-trait", @@ -2240,7 +2240,7 @@ dependencies = [ [[package]] name = "fabro-server" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "anyhow", "async-trait", @@ -2322,7 +2322,7 @@ dependencies = [ [[package]] name = "fabro-slack" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "fabro-http", "fabro-interview", @@ -2343,18 +2343,18 @@ dependencies = [ [[package]] name = "fabro-spa" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "rust-embed", ] [[package]] name = "fabro-static" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" [[package]] name = "fabro-store" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "async-trait", "bytes", @@ -2381,7 +2381,7 @@ dependencies = [ [[package]] name = "fabro-telemetry" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "anyhow", "base64", @@ -2407,7 +2407,7 @@ dependencies = [ [[package]] name = "fabro-template" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "anyhow", "fabro-util", @@ -2419,7 +2419,7 @@ dependencies = [ [[package]] name = "fabro-test" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "assert_cmd", "axum", @@ -2442,7 +2442,7 @@ dependencies = [ [[package]] name = "fabro-tracker" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "anyhow", "async-trait", @@ -2456,7 +2456,7 @@ dependencies = [ [[package]] name = "fabro-types" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "chrono", "clap", @@ -2477,7 +2477,7 @@ dependencies = [ [[package]] name = "fabro-util" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "anyhow", "console 0.15.11", @@ -2497,7 +2497,7 @@ dependencies = [ [[package]] name = "fabro-validate" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "fabro-graphviz", "fabro-model", @@ -2508,7 +2508,7 @@ dependencies = [ [[package]] name = "fabro-vault" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "chrono", "fabro-types", @@ -2520,7 +2520,7 @@ dependencies = [ [[package]] name = "fabro-workflow" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "anyhow", "assert_cmd", @@ -7298,7 +7298,7 @@ dependencies = [ [[package]] name = "twin-github" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "axum", "base64", @@ -7317,7 +7317,7 @@ dependencies = [ [[package]] name = "twin-openai" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" dependencies = [ "anyhow", "async-stream", diff --git a/Cargo.toml b/Cargo.toml index 989b8080a..f01038d80 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,7 +5,7 @@ resolver = "2" [workspace.package] edition = "2021" -version = "0.231.0-nightly.3" +version = "0.232.0-nightly.0" license = "MIT" [workspace.dependencies] From 6480c333ff87566443acb8f02c15443ccb0f911c Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Wed, 13 May 2026 07:27:26 -0400 Subject: [PATCH 2/3] docs: accept outside pull requests Switch the contribution policy from an issue-only model to welcoming outside PRs. Small fixes go straight to a PR; larger changes start with an issue or discussion. Co-Authored-By: Claude Opus 4.7 (1M context) --- CONTRIBUTING.md | 12 +++++------- README.md | 18 +++++------------- 2 files changed, 10 insertions(+), 20 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7228fbed8..9ac7fcd02 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -4,17 +4,15 @@ Thanks for your interest in contributing to Fabro! ## How to contribute -Fabro uses an **issue-based contribution model**. Instead of accepting outside pull requests, we accept bug reports and feature requests as GitHub Issues. +Outside contributions are welcome! Whether it's a bug fix, a new feature, documentation, or a typo -- we'd love your help making Fabro better. -1. **Open an issue** -- File an issue on [GitHub Issues](https://github.com/fabro-sh/fabro/issues) with a bug report or feature request. The more detail your issue contains, the easier it will be for us to address it quickly and successfully. -2. **We build it** -- A Fabro maintainer will follow our software development process to create a patch, supervising AI coding agents and workflows. -3. **You get credit** -- We will include you as a co-author on the commit which lands the change. - -See the [README](README.md#contributing-to-fabro) for more on why we use this model. +- **Bug fixes and small improvements** -- Send a pull request directly. No need to open an issue first. +- **Larger features or changes** -- Please open a [GitHub Issue](https://github.com/fabro-sh/fabro/issues) or start a [Discussion](https://github.com/fabro-sh/fabro/discussions) first so we can align on the approach before you invest significant time. +- **Questions** -- Open a Discussion or email [bryan@qlty.sh](mailto:bryan@qlty.sh). ## Development setup -If you are maintaining a fork, the instructions below will help you build and test locally. +The instructions below will help you build and test Fabro locally. ### Prerequisites diff --git a/README.md b/README.md index 7cc53e30a..73a1e4ffc 100644 --- a/README.md +++ b/README.md @@ -161,21 +161,13 @@ See the [deployment overview](https://docs.fabro.sh/administration/deployment) f ## Contributing to Fabro -Fabro uses an **issue-based contribution model**. Instead of accepting outside pull requests, we accept bug reports and feature requests as GitHub Issues. +Outside contributions are welcome! Whether it's a bug fix, a new feature, documentation, or a typo -- we'd love your help making Fabro better. -AI can rapidly write or edit large amounts of plausible-looking code. Accepting these patches from external sources opens up risks to security and quality. To mitigate these risks, we are tightly controlling the inputs into the software development process. +- **Bug fixes and small improvements** -- Send a pull request directly. +- **Larger features or changes** -- Open a [GitHub Issue](https://github.com/fabro-sh/fabro/issues) or start a [Discussion](https://github.com/fabro-sh/fabro/discussions) first so we can align on the approach. +- **Questions** -- Open a Discussion or email [bryan@qlty.sh](mailto:bryan@qlty.sh). -Contributions follow these steps: - -1. **Open an issue** -- File an issue with a bug report or feature request. The more detail your issue contains, the easier it will be for us to address it quickly and successfully. - -2. **We build it** -- A Fabro maintainer will follow our software development process to create a patch, supervising AI coding agents and workflows. - -3. **You get credit** -- We will include you as a co-author on the commit which lands the change. - -As a result, you get the feature you need, without needing to keep a fork in sync. - -If you need a capability which is not in-scope for Fabro, you always have the option to maintain a fork of Fabro as it is distributed under the MIT license. +See [CONTRIBUTING.md](CONTRIBUTING.md) for build instructions and development workflow. --- From 69ce83be4335a1bc2a0e12aec5b3a1f8be6d2897 Mon Sep 17 00:00:00 2001 From: David Julia Date: Wed, 13 May 2026 05:32:43 -0600 Subject: [PATCH 3/3] fix(slack): make per-button action_id unique (re #251 ) (#252) Heads up: [CONTRIBUTING.md](https://github.com/fabro-sh/fabro/blob/main/CONTRIBUTING.md) says you don't accept outside PRs, *"Instead of accepting outside pull requests, we accept bug reports and feature requests as GitHub Issues."* I filed the canonical bug report as **#251**, and that's where any actual discussion belongs. This PR is a courtesy ready-made diff in case it's useful to whoever supervises the AI workflow that lands this fix. Feel free to close it without comment; nothing is being asked of you here. I just thought it'd be useful to have a reference for what I did locally to fix it. ## What Two-file behaviour fix in `lib/crates/fabro-slack/`: every interview button on a multi-button gate now gets a Slack-unique `action_id`. Today they all share `"interview.answer"`, so Slack rejects the `chat.postMessage` with `invalid_blocks` and `SlackService::handle_event` silently drops the error. ## Why Multi-button Slack interview gates never reach Slack. Full reproducer, MITM-captured `invalid_blocks` response, and root-cause walkthrough are in **#251**. ## Diff shape - `blocks.rs`: each button gets a unique suffix. - `YesNo` / `Confirmation`: `interview.answer.yes` / `interview.answer.no` - `MultipleChoice`: `interview.answer.` (index, not raw key, to dodge Slack's 255-char `action_id` cap and any author-supplied charset surprises; selected key still rides in the button `value`) - `interaction.rs`: `parse_interaction` accepts both the legacy exact-prefix shape (in-flight buttons keep working across upgrade) and the new suffixed shape, via a pre-computed `ANSWER_ACTION_ID_PREFIX_DOT` constant so the parse hot path doesn't `format!` on every event. - Tests: +5 in `interaction.rs` (suffixed yes/no, suffixed multi-choice, legacy exact prefix, lookalike `interview.answers.yes` rejected, prefix-sync assertion). Updated the existing block-builder tests to assert uniqueness instead of the old single constant. One fixture each in `dispatch.rs` and `connection.rs` updated to the suffixed shape; one legacy fixture left in each to document backwards compatibility. 74/74 `fabro-slack` tests pass (was 69/69). `cargo +nightly-2026-04-14 fmt --check --all` and `cargo +nightly-2026-04-14 clippy -p fabro-slack --all-targets -- -D warnings` clean. ## Verified end-to-end Built a patched `fabro` binary, swapped it for the brew install, triggered a fresh multi-choice `Approve Plan` gate against a real Slack workspace, message rendered correctly in the configured channel with two clickable `[A] Approve` and `[R] Revise` buttons. Before the patch, the exact same gate produced zero Slack output and only the swallowed `invalid_blocks` was visible via MITM. ## Suggested follow-up (separate concern, not in this diff) The silent error swallow in `SlackService::handle_event` (`if let Ok(posted) = self.client.post_message(...)`) is what hid this bug. Worth logging at `WARN`. Mentioned in #251 as a separate item. ## Closes Closes #251 if you choose to land this directly. Otherwise this PR is just background material for the issue. --- lib/crates/fabro-slack/src/blocks.rs | 45 +++++++++-- lib/crates/fabro-slack/src/connection.rs | 2 +- lib/crates/fabro-slack/src/dispatch.rs | 2 +- lib/crates/fabro-slack/src/interaction.rs | 98 ++++++++++++++++++++++- 4 files changed, 136 insertions(+), 11 deletions(-) diff --git a/lib/crates/fabro-slack/src/blocks.rs b/lib/crates/fabro-slack/src/blocks.rs index 7ebc92be9..db143890f 100644 --- a/lib/crates/fabro-slack/src/blocks.rs +++ b/lib/crates/fabro-slack/src/blocks.rs @@ -4,11 +4,23 @@ use serde_json::{Value, json}; use crate::payload::{SlackActionPayload, encode_action_value}; -const ANSWER_ACTION_ID: &str = "interview.answer"; +pub(crate) const ANSWER_ACTION_ID_PREFIX: &str = "interview.answer"; 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"; +/// Build a Slack-unique `action_id` for an interview button. +/// +/// Slack requires `action_id`s to be unique within a single message and caps +/// them at 255 characters. The selected option is carried in the button +/// `value` payload, so the `action_id` only needs to be unique — it doesn't +/// have to encode the selection. Suffixes are short, fixed-shape tokens +/// (`yes`, `no`, or the option index) to avoid any character-set or length +/// concerns when option keys are author-supplied. +fn answer_action_id(suffix: &str) -> String { + format!("{ANSWER_ACTION_ID_PREFIX}.{suffix}") +} + fn text_block(text: &str) -> Value { json!({ "type": "section", @@ -48,11 +60,11 @@ pub fn question_to_blocks(run_id: &str, question_id: &str, question: &Question) button("Yes", &encode_action_value(&SlackActionPayload::Yes { run_id: run_id.to_string(), qid: question_id.to_string(), - }), ANSWER_ACTION_ID), + }), &answer_action_id("yes")), button("No", &encode_action_value(&SlackActionPayload::No { run_id: run_id.to_string(), qid: question_id.to_string(), - }), ANSWER_ACTION_ID), + }), &answer_action_id("no")), ] }); vec![section, actions] @@ -61,7 +73,8 @@ pub fn question_to_blocks(run_id: &str, question_id: &str, question: &Question) let elements: Vec = question .options .iter() - .map(|opt| { + .enumerate() + .map(|(idx, opt)| { button( &opt.label, &encode_action_value(&SlackActionPayload::Selected { @@ -69,7 +82,7 @@ pub fn question_to_blocks(run_id: &str, question_id: &str, question: &Question) qid: question_id.to_string(), key: opt.key.clone(), }), - ANSWER_ACTION_ID, + &answer_action_id(&idx.to_string()), ) }) .collect(); @@ -185,7 +198,23 @@ mod tests { let elements = actions["elements"].as_array().unwrap(); assert_eq!(elements.len(), 3); assert_eq!(elements[0]["text"]["text"], "Rust"); - assert_eq!(elements[0]["action_id"], ANSWER_ACTION_ID); + assert_eq!(elements[0]["action_id"], "interview.answer.0"); + assert_eq!(elements[1]["action_id"], "interview.answer.1"); + assert_eq!(elements[2]["action_id"], "interview.answer.2"); + // Slack requires action_id to be unique within a message. + let ids: std::collections::HashSet<&str> = elements + .iter() + .map(|e| e["action_id"].as_str().unwrap()) + .collect(); + assert_eq!(ids.len(), elements.len()); + // The option key remains in the button `value` payload so the server + // can still route the answer regardless of suffix scheme. + assert!( + elements[0]["value"] + .as_str() + .unwrap() + .contains("\"key\":\"rs\"") + ); assert!( elements[0]["value"] .as_str() @@ -217,7 +246,9 @@ mod tests { let actions = &blocks_json[1]; let elements = actions["elements"].as_array().unwrap(); - assert_eq!(elements[0]["action_id"], ANSWER_ACTION_ID); + assert_eq!(elements[0]["action_id"], "interview.answer.yes"); + assert_eq!(elements[1]["action_id"], "interview.answer.no"); + assert_ne!(elements[0]["action_id"], elements[1]["action_id"]); let value = elements[0]["value"].as_str().unwrap(); assert!(value.contains("\"run_id\":\"run-7\"")); assert!(value.contains("\"qid\":\"q-7\"")); diff --git a/lib/crates/fabro-slack/src/connection.rs b/lib/crates/fabro-slack/src/connection.rs index 9a863f2c6..afbf507a3 100644 --- a/lib/crates/fabro-slack/src/connection.rs +++ b/lib/crates/fabro-slack/src/connection.rs @@ -225,7 +225,7 @@ mod tests { "team": { "id": "T123" }, "user": { "id": "U123", "name": "ada" }, "actions": [{ - "action_id": "interview.answer", + "action_id": "interview.answer.yes", "type": "button", "value": "{\"kind\":\"yes\",\"run_id\":\"run-1\",\"qid\":\"q-1\"}" }] diff --git a/lib/crates/fabro-slack/src/dispatch.rs b/lib/crates/fabro-slack/src/dispatch.rs index 0c0ada6c9..548de5ea1 100644 --- a/lib/crates/fabro-slack/src/dispatch.rs +++ b/lib/crates/fabro-slack/src/dispatch.rs @@ -77,7 +77,7 @@ mod tests { "team": { "id": "T123" }, "user": { "id": "U123", "name": "ada" }, "actions": [{ - "action_id": "interview.answer", + "action_id": "interview.answer.yes", "type": "button", "value": "{\"kind\":\"yes\",\"run_id\":\"run-1\",\"qid\":\"q-1\"}" }] diff --git a/lib/crates/fabro-slack/src/interaction.rs b/lib/crates/fabro-slack/src/interaction.rs index ec8ca0006..4fc7bac1d 100644 --- a/lib/crates/fabro-slack/src/interaction.rs +++ b/lib/crates/fabro-slack/src/interaction.rs @@ -1,13 +1,23 @@ use fabro_interview::Answer; use serde_json::Value; +use crate::blocks::ANSWER_ACTION_ID_PREFIX; use crate::payload::{self, SlackActionPayload, SlackAnswerSubmission}; const MULTI_SELECT_BLOCK_ID: &str = "interview.checkboxes"; const MULTI_SELECT_ACTION_ID: &str = "interview.select"; -const ANSWER_ACTION_ID: &str = "interview.answer"; const MULTI_SELECT_SUBMIT_ACTION_ID: &str = "interview.submit"; +/// Buttons for the same question must each have a unique `action_id`, so the +/// outbound side stamps `interview.answer.` per element. This matches +/// either the exact prefix (legacy compatibility for messages posted before +/// the suffix scheme) or the suffixed form (current). +const ANSWER_ACTION_ID_PREFIX_DOT: &str = "interview.answer."; + +fn is_answer_action(action_id: &str) -> bool { + action_id == ANSWER_ACTION_ID_PREFIX || action_id.starts_with(ANSWER_ACTION_ID_PREFIX_DOT) +} + /// Parses a Slack interaction payload and returns a server-routable answer /// submission. pub fn parse_interaction(payload: &Value) -> Option { @@ -25,7 +35,7 @@ pub fn parse_interaction(payload: &Value) -> Option { let action_type = action["type"].as_str().unwrap_or("button"); let answer = match action_type { - "button" if action_id == ANSWER_ACTION_ID => match routed { + "button" if is_answer_action(action_id) => match routed { SlackActionPayload::Yes { .. } => Answer::yes(), SlackActionPayload::No { .. } => Answer::no(), SlackActionPayload::Selected { key, .. } => Answer { @@ -255,4 +265,88 @@ mod tests { }); assert!(parse_interaction(&payload).is_none()); } + + /// Suffixed `action_id`s (per-button uniqueness for Slack) must still + /// route to the correct answer. + #[test] + fn parse_suffixed_yes_action_id() { + let payload = serde_json::json!({ + "type": "block_actions", + "team": { "id": "T123" }, + "user": { "id": "U123", "name": "ada" }, + "actions": [{ + "action_id": "interview.answer.yes", + "type": "button", + "value": "{\"kind\":\"yes\",\"run_id\":\"run-1\",\"qid\":\"q-1\"}" + }] + }); + let submission = parse_interaction(&payload).unwrap(); + assert_eq!(submission.answer.value, AnswerValue::Yes); + } + + #[test] + fn parse_suffixed_multiple_choice_action_id() { + // `interview.answer.` is what `question_to_blocks` now produces + // for multiple_choice questions. + let payload = serde_json::json!({ + "type": "block_actions", + "team": { "id": "T123" }, + "user": { "id": "U123", "name": "ada" }, + "actions": [{ + "action_id": "interview.answer.2", + "type": "button", + "value": "{\"kind\":\"selected\",\"run_id\":\"run-1\",\"qid\":\"q-1\",\"key\":\"py\"}" + }] + }); + let submission = parse_interaction(&payload).unwrap(); + assert_eq!( + submission.answer.value, + AnswerValue::Selected("py".to_string()) + ); + } + + /// Legacy `action_id` without a suffix must still parse so messages + /// posted by older Fabro builds remain clickable after upgrade. + #[test] + fn parse_legacy_unsuffixed_action_id() { + let payload = serde_json::json!({ + "type": "block_actions", + "team": { "id": "T123" }, + "user": { "id": "U123", "name": "ada" }, + "actions": [{ + "action_id": "interview.answer", + "type": "button", + "value": "{\"kind\":\"yes\",\"run_id\":\"run-1\",\"qid\":\"q-1\"}" + }] + }); + let submission = parse_interaction(&payload).unwrap(); + assert_eq!(submission.answer.value, AnswerValue::Yes); + } + + /// Action ids that merely share a prefix but are not the answer family + /// must not be misrouted (no false-positive prefix match). + #[test] + fn rejects_lookalike_action_id() { + let payload = serde_json::json!({ + "type": "block_actions", + "team": { "id": "T123" }, + "user": { "id": "U123", "name": "ada" }, + "actions": [{ + "action_id": "interview.answers.yes", + "type": "button", + "value": "{\"kind\":\"yes\",\"run_id\":\"run-1\",\"qid\":\"q-1\"}" + }] + }); + assert!(parse_interaction(&payload).is_none()); + } + + /// The dotted prefix constant must stay in sync with the canonical + /// prefix so outbound and inbound never drift. + #[test] + fn dotted_prefix_constant_matches_canonical_prefix() { + assert_eq!( + ANSWER_ACTION_ID_PREFIX_DOT, + format!("{ANSWER_ACTION_ID_PREFIX}.") + ); + } }