From 402045df43f9a740c526b87acd03548bd3da8d7e Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 29 Oct 2026 16:25:32 +1100 Subject: [PATCH] test: dedupe collect_until_eose by event id and uncap two shell tests Fixes two CI flakes from the 21-28 Oct 2026 audit. Both are test-layer changes, with no product code. Not included: the buzz-terminal passwd_shell() getpwuid race (env_fence_tests::resolve_shell_falls_through_to_passwd_not_the_default, run 38590095289). Open PR #7687 (fixes #7589) already replaces getpwuid with nix::unistd::User::from_uid and adds a concurrent-read test. Neither origin/main (eda26b942) nor an open PR covers the two fixes below. #8636 and #7976 edit buzz-test-client/src/lib.rs, and #7607 and #7915 edit buzz-dev-mcp/src/shell.rs, but none of them changes collect_until_eose or these timeouts. 1. Relay E2E: the same event is delivered twice before EOSE. Test: e2e_project::test_project_tombstone_deletes_coordinate_and_ spares_members (e2e_project.rs:338 "project should be live before deletion left: 2 right: 1") Run: 40408678412#1, job 121879630394 (PR #8620, schema-only) Cause: the EVENT handler sends OK, then fans out in a spawned task. handle_req registers the live subscription before its history query, and req.rs's seen_ids dedups history rows only. When the fan-out task runs after the REQ is registered, the event arrives once live and once from history, both before EOSE. collect_until_eose kept every EVENT frame. The helper has 96 call sites. Change: collect_until_eose keeps the first copy of each event id, as production clients do (buzz-acp drops repeats with its seen_ids TwoGenDedup; the CLI reads /query, which has no live phase). The doc comment says a test of the relay's own dedup must read frames with recv_event. Pre-check: no test relies on the relay's own history dedup. Four e2e REQs carry two filters: two expect CLOSED, and the other two only assert that an event is present. No e2e test sends a raw REQ. Every exact-count assertion uses one filter, and the NIP-33 "only newest remains" ones would see a failed replacement as two different ids, which this dedup keeps. Regression test: buzz-test-client tests::collect_until_eose_keeps_one_copy_of_each_event_id. A fake relay (TcpListener plus accept_async) sends raced, other on another subscription, other, raced, then EOSE. Before -> after: FAIL (ids [raced, other, raced]) -> PASS. E2E against an isolated local relay (own database, Redis /9, ports 3017/8097/9297, fresh community UUID): e2e_persona 24, e2e_team_catalog 9, e2e_nostr_interop 25, e2e_thread_roots 1, e2e_project 7, e2e_event_reminder 29, e2e_author_only_deletion 15, e2e_team 3, e2e_managed_agent 5, e2e_long_form 8, e2e_user_status 5 and e2e_workflow_deletion 4 all pass. e2e_relay passes 62/63: test_unarchive_emits_member_added_notification times out in recv_event on untouched code too (alone: 4/5 failures with the fix, 3/5 with the original helper), and CI's Relay E2E lane does not select it. conformance_multitenant fails the same 10 tests before and after (8 todo!() stubs, the NIP-11 enumeration oracle, a workflow trigger POST 400). The original race was not reproduced locally. Follow-up (out of scope): make the relay deliver each event id at most once per subscription. Keep a bounded per-subscription set of ids sent from history, freed on CLOSE, keeping the live/history overlap so no event is dropped in the gap. That also covers a late fan-out after EOSE that repeats a history event, which this helper can't see, and gives req.rs's OR-overlap dedup its first test. 2. Windows buzz-dev-mcp shell::tests::basic_echo Test: buzz-dev-mcp src/shell.rs basic_echo (:1026 "left: Number(124) right: 0") Run: 39675698081#1, job 119485363399 (fix-push-delivery-polling), green on re-run Cause: 124 is the shell tool's own timeout exit code. basic_echo capped the shell at 5 s, and it is one of the first tests in the process to start Git Bash. On a cold Windows runner that first start can take more than 5 s. The test checks echo output; timeout_fires (150 ms) covers timeouts. Change: timeout_ms: None in basic_echo and in workdir_is_honored, which has the same cap and the same cold-start exposure. None falls back to the production DEFAULT_TIMEOUT_MS (120 s), so passing runs are no slower. Verification: macOS only. The failure is Windows-only, so the Windows Rust lane is the check. Not reproduced. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- crates/buzz-dev-mcp/src/shell.rs | 6 ++- crates/buzz-test-client/src/lib.rs | 67 +++++++++++++++++++++++++++++- 2 files changed, 69 insertions(+), 4 deletions(-) diff --git a/crates/buzz-dev-mcp/src/shell.rs b/crates/buzz-dev-mcp/src/shell.rs index 028e8ec8da5..33577338428 100644 --- a/crates/buzz-dev-mcp/src/shell.rs +++ b/crates/buzz-dev-mcp/src/shell.rs @@ -1016,7 +1016,9 @@ mod tests { ShellParams { command: "echo hello".into(), workdir: None, - timeout_ms: Some(5_000), + // The production default: a cold Git Bash start on a Windows + // runner can take more than 5 s. `timeout_fires` covers timeouts. + timeout_ms: None, }, CancellationToken::new(), ) @@ -1064,7 +1066,7 @@ mod tests { ShellParams { command: "pwd".into(), workdir: Some(sub.display().to_string()), - timeout_ms: Some(5_000), + timeout_ms: None, }, CancellationToken::new(), ) diff --git a/crates/buzz-test-client/src/lib.rs b/crates/buzz-test-client/src/lib.rs index 89bff3020df..e934db60c58 100644 --- a/crates/buzz-test-client/src/lib.rs +++ b/crates/buzz-test-client/src/lib.rs @@ -3,6 +3,7 @@ //! Minimal NIP-01 WebSocket test client for the Buzz relay. +use std::collections::HashSet; use std::time::Duration; use nostr::{Event, EventBuilder, Filter, Keys, Kind, Tag}; @@ -175,7 +176,14 @@ impl BuzzTestClient { Ok(self.inner.next_event(timeout_dur).await?) } - /// Collects all events for `sub_id` until EOSE is received, waiting up to `timeout_dur`. + /// Collects the events for `sub_id` until EOSE is received, waiting up to + /// `timeout_dur`, keeping the first copy of each event id. + /// + /// The relay registers a REQ's live subscription before it queries + /// history, so an event whose live fan-out runs late can arrive twice + /// before EOSE. Production clients drop the repeat, and so does this + /// helper. A test of the relay's own per-subscription dedup must read + /// frames with [`Self::recv_event`] instead. pub async fn collect_until_eose( &mut self, sub_id: &str, @@ -183,6 +191,7 @@ impl BuzzTestClient { ) -> Result, TestClientError> { let deadline = tokio::time::Instant::now() + timeout_dur; let mut events = Vec::new(); + let mut seen = HashSet::new(); loop { let remaining = deadline @@ -198,7 +207,9 @@ impl BuzzTestClient { subscription_id, event, } if subscription_id == sub_id => { - events.push(*event); + if seen.insert(event.id) { + events.push(*event); + } } RelayMessage::Eose { subscription_id } if subscription_id == sub_id => { return Ok(events); @@ -348,4 +359,56 @@ mod tests { "missing h tag" ); } + + /// The relay registers a REQ's live subscription before its history + /// query, so an event whose fan-out task runs late arrives once live and + /// once from history, both before EOSE. + #[tokio::test] + async fn collect_until_eose_keeps_one_copy_of_each_event_id() { + use futures_util::SinkExt; + use tokio_tungstenite::tungstenite::Message; + + let keys = Keys::generate(); + let raced = EventBuilder::new(Kind::TextNote, "raced") + .sign_with_keys(&keys) + .unwrap(); + let other = EventBuilder::new(Kind::TextNote, "other") + .sign_with_keys(&keys) + .unwrap(); + let frames = [ + json!(["EVENT", "sub", raced]), + json!(["EVENT", "elsewhere", other]), + json!(["EVENT", "sub", other]), + json!(["EVENT", "sub", raced]), + json!(["EOSE", "sub"]), + ]; + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind fake relay"); + let address = listener.local_addr().expect("fake relay address"); + let relay = tokio::spawn(async move { + let (stream, _) = listener.accept().await.expect("accept client"); + let mut ws = tokio_tungstenite::accept_async(stream) + .await + .expect("fake relay handshake"); + for frame in frames { + ws.send(Message::Text(frame.to_string().into())) + .await + .expect("send frame"); + } + ws + }); + + let mut client = BuzzTestClient::connect_unauthenticated(&format!("ws://{address}")) + .await + .expect("connect to fake relay"); + let events = client + .collect_until_eose("sub", Duration::from_secs(5)) + .await + .expect("collect until EOSE"); + let ids: Vec<_> = events.iter().map(|event| event.id).collect(); + assert_eq!(ids, [raced.id, other.id]); + drop(relay.await.expect("join fake relay")); + } }