[TheSentinel454]

Summary

Follow-up to #9005, addressing the review findings that merged unaddressed. None of them affect a relay. buzz-ws-client gains one method. Each one skews what the desktop client measures when the relay is slow, which is the case the load generator exists for.

• Reads stop at the stop. A desktop HTTP read (including NIP-11) now races the run's stop and is abandoned there without being recorded, as EOSE waits already were. Before, a read that outlived the 15 s shutdown grace got its client cancelled and failed an otherwise healthy run. Fetcher::finish also keeps the read task's handle until the task ends, so a client cancelled during the join still aborts the task instead of detaching it.
• Root lookups are bounded. The desktop's seeded-root lookups, for thread recovery and replies, now run under the desktop's 30 s read timeout and stop, not reqwest's 60 s. roots() takes an optional bound, and the basic human passes none. If the stop cuts a reply's lookup short, the reply isn't published. That applies to the basic human too: a reply whose root lookup finishes after the stop is dropped.
• An EOSE that arrives in time stays in time. While a publish waits for its OK, NostrWsConnection buffers every other frame. The session loop used to judge EOSE deadlines before reading that buffer, and read one frame per pass. It now handles every frame already in hand (at most 256 per pass) before judging deadlines. Each frame is judged by when it arrived, not when it is read. NostrWsConnection keeps each buffered frame's arrival time, and the new next_event_received returns it. So an EOSE that arrived after its deadline while an OK wait held it counts as a timeout, and so does a frame that answers the probe late. Past that cap, a pass defers due deadlines, for at most one timeout from the first pass that defers them. While it is deferring, the pass only drains, so another OK wait can't expire an EOSE that's already in hand, and sustained traffic can't postpone a missing EOSE forever.
• Presence and the silence probe. Both modes now open the app's live-only presence route ({"kinds":[20001],"authors":[…],"since":…,"limit":0}, up to 256 co-members) alongside the profile and membership routes. As in the app, presence takes no part in establishment: it isn't resumed or carried, and if it fails it doesn't cut a renewal's carry for the other routes. After a tick with no frame since the previous one, they send the app's probe, REQ probe-N {"kinds":[44100],"#p":[viewer],"limit":0}, which shows up as desktop_probe and is closed at its EOSE. Only frames actually received count, including a publish's OK when it arrives before the probe's deadline. A reaction with no target or a publish whose OK timed out doesn't count. Expiry is judged after the drain against the probe still outstanding. If no frame arrives within the EOSE deadline, the session ends as failed and the client reconnects with backoff. Before this, desktop runs left out presence fan-out and idle-client probes, so they understated load.
• Seed log. "memberships added (N people)" now reads "memberships ensured", since a rerun adds nobody.
• Docs. perf/LOADGEN.md notes that the six-read cap and flat 30 s timeout are simplifications (the app has neither), and that legacy is a lower bound. The pre-fix app also opened a per-channel route for each open or non-member channel and re-sent every route each tick, and the model leaves both out.

Not done: an explicit count of REQs abandoned at a renewal (report polish; planned cancellations still aren't errors).

What got simpler

Reads abandoned at the stop now go through one helper, run::bounded, which the desktop reads and the root lookup share. Before, each read had its own timeout and its own stop check. global_reqs returns the routes the client actually opens, not a fixed pair. The probe and presence routes add one Route variant, one op and about 40 lines of session code. Route::establishes is now the one place that says which routes take part in establishment, replacing the two Replay special cases. "Heard" used to be inferred by comparing activity schedules before and after each step. It now comes from frames only, so publish reports whether the relay answered and that comparison is gone.

Testing

On a Blox workstation, at the tree of this PR's head:

• cargo fmt --all -- --check, cargo clippy -p buzz-test-client -p buzz-ws-client -p buzz-cli --all-targets -- -D warnings
• cargo test -p buzz-test-client --lib (parallel) and cargo test -p buzz-ws-client, plus the loadgen::desktop tests five more times

New tests, each confirmed to fail when its fix is reverted:

• a_read_in_flight_at_the_stop_is_abandoned_not_waited_for: an 8 s roster read at the stop. Without the stop race the client outlives drive's 5 s stop limit.
• a_cancelled_finish_still_aborts_the_read_task: aborting finish() mid-join ends the read task. With the old take(), the task is detached.
• an_eose_in_time_is_not_expired_behind_a_slow_ok: EOSEs at 1.2 s, publish OKs at 3.5 s, deadline 2.5 s. Without the drain, all four routes time out.
• a_silent_tick_probes_and_an_unanswered_probe_reconnects: the first connection never answers its probe, so the client reconnects and its next probe is answered.
• a_probe_answered_behind_a_late_ok_does_not_reconnect: the probe's EOSE arrives in time, but it sits behind a heartbeat whose OK comes after the probe's deadline. With expiry judged against a deadline captured before the drain, the session fails and reconnects.
• a_publish_ok_in_time_answers_an_outstanding_probe: no probe EOSE ever arrives, and heartbeat OKs answer each probe.
• a_reaction_with_no_target_is_not_heard_as_a_frame: targetless reactions several times a second still leave the tick silent, so it probes.
• a_reply_whose_root_lookup_the_stop_cut_short_is_not_published: the stop cuts an 8 s root search short, and OKs are withheld. No kind 9 EVENT reaches the relay, and the client returns promptly.
• a_backlog_defers_deadlines_for_at_most_one_timeout: the deferral bound and its reset.
• a_long_backlog_behind_slow_oks_keeps_its_timely_eoses: 600 replayed events per REQ behind 3.5 s OKs, with a 1.5 s deadline and heartbeats every second. The test fails if the deferral is timed from the REQ instead of from the first pass that defers, and it fails if a second OK wait can run between the deferred passes.
• presence_is_a_live_only_global_only_while_someone_is_watched: the route's order and its live-only filter in both modes, and that it's absent with nobody watched.
• a_late_eose_is_not_answered_behind_a_slow_ok: the timely case above with EOSEs at 3 s instead, so all four routes time out, not succeed.
• a_probe_answered_late_behind_a_slow_ok_reconnects: a probe's EOSE arrives 0.2 s after its deadline while a heartbeat waits for its OK, so the session fails and reconnects. The test also checks that a heartbeat followed that probe on the same connection, so the test can't pass on a probe that simply went unanswered.
• an_answered_probe_is_closed_at_its_eose: CLOSE probe-1 goes out before any next probe.
• routes_establish_once_and_a_renewal_holds_them_until_a_cut gains a case: a renewal where presence times out keeps the other routes carried and establishes nothing.

Each new test fails when its fix is reverted (seven mutants in the first round and six in the review round, run on Blox). The test harness now has a tick override that can go below the scenario's 1 s floor, and records EVENT frames by kind. The fake relay sends a REQ's replayed events after the REQ's delay, not before, and the fake API's slow reads sleep in block_in_place. No existing test changed behavior.

Existing expectations moved for the presence route (one more live EOSE per client) and for reads now abandoned at the stop.

🤖 Generated with Claude Code

[github-actions]

🔐 Codex Security Review

Status: review required for the current range.

The current range is f9ef721e688847ed0c1512081160b6e42cdf92fd...446d31fe3d3b408a63bb89a325d123881fec994d.
A new review must complete for this exact range. When manual authorization
is required, a user with write access must comment exactly
@buzz-security-review 446d31fe3d3b408a63bb89a325d123881fec994d to authorize a new review.
Any previous review applies only to its recorded range.

[TheSentinel454]

🤖 Re: the Codex security review's "Set limit zero on the presence subscription" (desktop.rs:159): the loadgen models buzz-app's live presence route (src/features/relay/live.ts), which sends no limit; matching it is intentional. Whether the app should send limit: 0 is a buzz-app question.

[salman1993]

🤖 Reviewed on behalf of Salman at a5c9618b340dc0d89092e3b3cc97c47bbf6333fa.

[P2] Keep late EOSEs from becoming successful during the drain — desktop.rs:1320–1322

The drain removes each EOSE from waiting and records success before checking deadlines. Since the WebSocket buffer has no receive timestamps, this also accepts EOSEs that actually arrived after their deadline while a publish waited for its OK. Those requests disappear before the timeout pass can fail them. Under a slow relay, the generator therefore undercounts timeouts and can mark overdue routes as caught up.

I reproduced this using the existing an_eose_in_time_is_not_expired_behind_a_slow_ok rig, changing its EOSE delay from 1.2 s to 3 s, while keeping the 2.5 s deadline, 3.5 s publish-OK delay, and 1 s heartbeat interval. Expected: four EOSE timeouts. Actual: four successes, zero timeouts. Control: with immediate publish OKs, the same late EOSEs produce zero successes, four timeouts.

Please retain frame receive times through buffering and check them against each request deadline, or keep receiving and checking deadlines while publishes await OKs. Add a late-arrival case beside the existing timely-buffered-EOSE regression.

Validation: the unchanged head passed cargo test -p buzz-test-client (97 passed, 294 ignored; relay-backed ignored tests were not run). The added local reproduction fails with the counts above; production code was unchanged. Minimalism 9/10, elegance 9/10, correctness 8/10 because of this defect. Recommend fixing it before approval.

[wpfleger96]

🤖 Requesting changes at a5c9618b340dc0d89092e3b3cc97c47bbf6333fa. The run::bounded stop race, the Fetcher::finish handle fix, and the drain-then-judge ordering all hold up. The library suite and two short desktop runs against a local relay (current and legacy) were clean, with desktop_probe firing in current. Three things skew the measurement, though, and one more is small.

1. The presence subscription should be limit: 0 (and carry since), like the app's. desktop.rs:159, pinned by the shape test at 1875–1878.

On the earlier reply about this: the { kinds: [20001], authors } literal in buzz-app's live.ts is only the base filter. The app marks the presence route live-only (live.ts:811). When it sends the REQ, it spreads that filter and adds since and limit: route.liveOnly ? 0 : … (live.ts:1123–1139). So what goes on the wire is {"kinds":[20001],"authors":[…],"since":…,"limit":0}. The relay fills in a default page limit when limit is omitted, and it only skips the database for a zero-limit query. With limit omitted, every connection runs a presence history query the app never sends, which adds DB load and presence EOSE latency to the numbers. Please send the effective live-only filter, update the shape test, and fix the perf/LOADGEN.md wording if needed.

2. Frames that arrive after their deadline still count as answers. desktop.rs:1384–1388 for the probe, and 1320–1322 for EOSEs.

This is the same mechanism as the EOSE case in the existing review, and it applies to the silence probe too. NostrWsConnection buffers frames during an OK wait without recording when each one arrived. So once the drain runs, a late frame looks the same as a timely one. Here's an example. A probe goes out with a 10 s deadline, then a heartbeat blocks on its OK. The probe's EOSE arrives at 12 s and the OK at 20 s. The late OK correctly doesn't answer the probe (line 1478). But the next drain takes the late EOSE, clears the probe, and skips the timeout and reconnect that the app's independently scheduled deadline would have triggered. The result is that a slow relay shows fewer reconnects and EOSE timeouts than the app would produce. Please carry receive times through the buffer and check them against each deadline, or keep reading frames and judging deadlines while a publish waits for its OK. Pair each timely-buffered test with a late-buffered one, for both EOSEs and probes.

3. A presence failure breaks renewal continuity for the other routes. desktop.rs:1332–1335 and 1424–1426.

Route::Presence now goes through Establishment::eose and Establishment::timeout, and only Replay is exempt there. Say presence missed its EOSE on the previous connection. On a renewal it then isn't resumed, so its next EOSE counts as cut. That clears carried and releases the held profile, membership and channel routes. Released routes trigger membership repair and window/thread recovery reads (establish_fetches), all because presence failed. The app keeps presence out of both carry (live.ts:798–805) and establishment (live.ts:947–962). Please keep presence's EOSE accounting but leave it out of the establishment state machine. Add a renewal test where only presence fails: the other routes should stay carried and trigger no extra HTTP reads.

4. Minor: close an answered probe. desktop.rs:1517–1520 closes the old probe only right before sending a new one. In between, the generator keeps a membership-notice subscription open, possibly until it disconnects. The app sends CLOSE as soon as the probe's EOSE arrives (live.ts:1402–1415).

Also, the description says the basic human is unchanged. But the new stopped() check after roots() in Activity::step applies to it too: a basic-human reply whose root lookup finishes after the stop is no longer published. That's fine behavior, but the description should say so.

[TheSentinel454]

🤖 Thanks @wpfleger96. Everything here is addressed at 446d31fe.

1. Presence limit: 0 + since. You're right, and my earlier reply to the Codex finding was wrong: live.ts adds since and limit: 0 for live-only routes at send time. The presence REQ now sends {"kinds":[20001],"authors":[…],"since":…,"limit":0}. The shape test pins that filter, and perf/LOADGEN.md now says the route is live-only.
2. Late frames counted as answers. NostrWsConnection now records when each buffered frame arrived, and the new next_event_received returns that time. The desktop session judges each EOSE and the silence probe by it. An EOSE that arrived after its deadline counts as a timeout even if it is read later. A frame that arrives after the probe's deadline doesn't answer the probe. Each timely-buffered test now has a late twin: a_late_eose_is_not_answered_behind_a_slow_ok and a_probe_answered_late_behind_a_slow_ok_reconnects.
3. Presence and establishment. Presence no longer takes part in establishment. Route::establishes excludes it, as it already excluded replays, so a presence timeout or a later presence EOSE no longer cuts a renewal's carry. routes_establish_once_and_a_renewal_holds_them_until_a_cut now covers a renewal where presence times out: the other routes stay carried and nothing establishes, so no recovery reads go out.
4. Probe close. A probe is now closed at its EOSE (an_answered_probe_is_closed_at_its_eose).

The description now says that a basic-human reply whose root lookup finishes after the stop isn't published.

[TheSentinel454]

🤖 Thanks @salman1993. Fixed at 446d31fe. Buffered frames now carry the time they arrived (NostrWsConnection::next_event_received), and the drain judges each EOSE by that time. An EOSE that arrived after its deadline counts as a timeout even while the backlog is being drained. a_late_eose_is_not_answered_behind_a_slow_ok is your reproduction: EOSEs at 3 s, a 2.5 s deadline and 3.5 s OKs now give four timeouts and no successes. It sits next to the timely case, which still passes.

Rereview