fix(cli): preserve network errors in mention preflight

[olympusbuildz]

Problem

buzz messages send against an unreachable relay reports network_error / retryable: true / exit 2 when the body has no mention processing. The same outage with an @ outside a code region (or with --mention) is rewritten as:

{"error":"error","message":"could not load channel membership for mention preflight","retryable":false}

exit 4. Wrappers that honor retryable permanently drop attention-seeking messages.

Root cause

fetch_events in crates/buzz-cli/src/commands/messages.rs did client.query(...).await.ok()?, discarding CliError::Network. resolve_content_mentions then mapped bare None to CliError::Other, which is never retryable.

Three unrelated conditions collapsed into one membership-looking message: transport failure, non-array body, and a channel with no kind 39002 event.

Fix

• fetch_events → Result<Vec<_>, CliError> (same shape as fetch_event in this file)
• fetch_member_pubkeys → Result<Option<Vec<_>>, CliError> where Ok(None) means no 39002 event
• Missing membership → NotFound naming the channel
• Transport/parse failures propagate unchanged

Why it matters

retryable drives automation. Inverting it only for @ bodies is a silent drop class on the messages that were meant to notify someone.

Test plan

cargo test -p buzz-cli --lib commands::messages::tests
# 32 passed @ e21e132dbe

Named regression: mention_preflight_preserves_network_error_when_relay_unreachable — BuzzClient to http://127.0.0.1:1, body with @ and explicit-mention path both stay CliError::Network, retryable, exit 2; plain body still skips preflight.

Risk

Blast radius is CLI mention preflight only. Empty membership is now not_found instead of generic error — more accurate for operators.

Closest work

none found on fetch_events / mention-preflight retryable inversion (open PRs on messages.rs cover empty content, NUL, etc., not this path).

Fixes #6555

[cristiansotogarciaxatech]

I filed #6555. This is the right fix and it is the same shape I had going on my own branch, so I am not opening a competing PR. I verified yours instead.

Two things stand between this and a merge.

1. It fails the Lint gate at e21e132d

ci.yml runs just fmt-check, which is cargo fmt --all -- --check. Your head fails it in two places, both introduced by this PR.

• crates/buzz-cli/src/commands/messages.rs:302, the format! inside fetch_events has to wrap across three lines
• crates/buzz-cli/src/commands/messages.rs:1118, the let with_at = resolve_content_mentions(...) binding has to wrap

CI has not told you this yet. All three workflow runs on your head are sitting at action_required, so DCO is the only check that has actually executed.

Run on your unmodified tree, one workflow file added and nothing else touched:
https://github.com/cristiansotogarciaxatech/buzz/actions/runs/32636961252

Same command on main at e236329 comes back clean, so it is this PR and not pre-existing drift. Both used rustfmt 1.9.0-stable, which is what the repo's own rust-toolchain.toml pin resolves to under hermit.

cargo fmt --all clears it.

2. Three behaviour changes land with no test

Your regression test covers the unreachable relay on the @ path and the --mention path, plus the skip. That part is solid. Underneath it this PR changes three other outcomes and nothing pins any of them.

• a retryable relay status during the preflight, 429 502 503 504, now propagates instead of collapsing into error
• an empty membership result is now not_found, which moves the exit code from 4 to 1
• a 200 carrying a non-array body now says what was actually wrong instead of blaming membership

I ran a wider set against your exact head. 38 passed, 0 failed, so your fix already gets all three right:
https://github.com/cristiansotogarciaxatech/buzz/actions/runs/32636977265

One caveat on that run. I had to apply cargo fmt first to get past item 1. None of your logic was touched.

The same six tests against unfixed production code fail 5 of 6, and the no-mentions control still passes:
https://github.com/cristiansotogarciaxatech/buzz/actions/runs/32636645840

That is what makes them regression tests rather than decoration. An exit code moving from 4 to 1 is the kind of thing a wrapper notices at the worst possible moment, so I would rather it were nailed down.

Take them if you want them. This is the exact code that ran green above.

Tests for the three uncovered paths

    use axum::body::Body;
    use axum::http::{Response, StatusCode};
    use axum::routing::post;
    use axum::Router;
    use tokio::net::TcpListener;

    const CHANNEL: &str = "123e4567-e89b-12d3-a456-426614174000";

    fn send_params(channel_id: &str, content: &str, mentions: Vec<String>) -> SendMessageParams {
        SendMessageParams {
            channel_id: channel_id.to_string(),
            content: content.to_string(),
            kind: None,
            reply_to: None,
            broadcast: false,
            files: vec![],
            mentions,
        }
    }

    /// Serve one fixed response body on `POST /query`.
    async fn query_server(status: StatusCode, body: &'static str) -> String {
        let app = Router::new().route(
            "/query",
            post(move || async move {
                Response::builder()
                    .status(status)
                    .header("content-type", "application/json")
                    .body(Body::from(body))
                    .unwrap()
            }),
        );
        let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
        let addr = listener.local_addr().unwrap();
        tokio::spawn(async move { axum::serve(listener, app).await.unwrap() });
        format!("http://{addr}")
    }

    /// An overloaded relay is retryable too, and the preflight must say so.
    #[tokio::test]
    async fn relay_503_during_the_preflight_stays_retryable() {
        let url = query_server(StatusCode::SERVICE_UNAVAILABLE, "relay is overloaded").await;
        let client = BuzzClient::new(url, Keys::generate(), None, None).unwrap();

        let error = cmd_send_message(&client, send_params(CHANNEL, "ping @somebody", vec![]))
            .await
            .unwrap_err();

        assert!(
            matches!(error, CliError::Relay { status: 503, .. }),
            "expected relay 503, got {error:?}"
        );
        assert!(
            is_retryable_error(&error),
            "503 is retryable, got {error:?}"
        );
    }

    /// The relay answered. The channel genuinely has no kind 39002 event.
    /// That is a real membership fact and it gets its own error, separate
    /// from any lookup failure.
    #[tokio::test]
    async fn a_channel_with_no_membership_event_is_reported_as_not_found() {
        let url = query_server(StatusCode::OK, "[]").await;
        let client = BuzzClient::new(url, Keys::generate(), None, None).unwrap();

        let error = cmd_send_message(&client, send_params(CHANNEL, "ping @somebody", vec![]))
            .await
            .unwrap_err();

        assert!(
            matches!(error, CliError::NotFound(_)),
            "expected not found, got {error:?}"
        );
        assert!(
            error.to_string().contains(CHANNEL),
            "the error must name the channel, got {error}"
        );
        assert!(
            !is_retryable_error(&error),
            "a missing membership record will not fix itself on retry"
        );
    }

    /// A 200 carrying something that is not a JSON array is a real defect
    /// somewhere, and it must not be mistaken for an empty channel.
    #[tokio::test]
    async fn a_non_array_query_response_is_reported_as_a_bad_response() {
        let url = query_server(StatusCode::OK, r#"{"events":[]}"#).await;
        let client = BuzzClient::new(url, Keys::generate(), None, None).unwrap();

        let error = cmd_send_message(&client, send_params(CHANNEL, "ping @somebody", vec![]))
            .await
            .unwrap_err();

        assert!(
            matches!(error, CliError::Other(_)),
            "expected a generic error, got {error:?}"
        );
        assert!(
            error.to_string().contains("not a JSON array"),
            "the error must say what was wrong with the body, got {error}"
        );
    }

axum is already a dev-dependency of buzz-cli, so nothing new is needed in Cargo.toml. These call cmd_send_message, which means you also need cmd_send_message and SendMessageParams in the use super::{...} list.

Either way, thanks for turning this around quickly. I care that it lands, not who lands it.