[chappie-daemon]

Three concurrency defects in nostream-relay, found after the relay handlers landed in #947. Each was cut as its own issue and each was re-checked against the source before the fix.

Closes #950
Closes #951
Closes #952

What was wrong

• relay: a re-REQ can cancel the read that replaced it #950 — connection.rs. end_read removed the in-flight entry by subscription id, unconditionally. begin_read aborts the read it replaces, so when the replaced read finished, its own end_read removed the replacement's cancellation sender. The replacement's watch::Receiver then saw a closed channel, took the cancellation arm, and returned without EOSE. Client-visible as: re-REQ a subscription with new filters and it can go silent. This is new in the port — the capture has no per-id table and does not abort on a plain re-REQ.
• relay: CLOSE during an invite mint is lost, and frames follow the close #951 — handlers/subscribe.rs. req awaited the NIP-43 invite mint before any read was registered, so a CLOSE arriving in that window found nothing to abort: the handler then emitted the invite EVENT and streamed the whole stored read to EOSE to a subscription the client had closed.
• relay: the outbound frame queue is unbounded, so a slow client becomes relay memory #952 — sink.rs, relay.rs. FrameSink::channel() was an unbounded channel with no producer back-pressure, and its only consumer is the same select loop that performs the socket write. A stored read of large events could queue without limit while a client drained at link speed (recon measured 405 MiB for 200 000 queued frames), and once a write blocked, the loop could not poll the liveness tick at all.

The change

• Read ownership by generation (relay: a re-REQ can cancel the read that replaced it #950). The table becomes Mutex<InFlightReads>: a connection-wide monotonic next_generation: u64 and by_subscription: HashMap<String, (u64, watch::Sender<bool>)>. begin_read returns an InFlightRead handle carrying the id, its generation and the receiver; end_read takes it by value and removes the entry only while the stored generation still matches, so a replaced read's last act is a no-op. abort_read, read_in_flight and close keep their signatures and behaviour. One mutex as before, no allocation outside it, no .await in impl Connection.
• Cancellation registered before the mint's only await (relay: CLOSE during an invite mint is lost, and frames follow the close #951). begin_read moves out of fetch_and_send and into handle, ahead of maybe_issue_invite_code(..).await; the cancellation is checked before find_by_filters, so a CLOSE that landed during the mint ends the subscription with no stored event and no EOSE. The invite EVENT is deliberately still emitted — that is the capture's own behaviour (subscribe-message-handler.ts:111), and parity is the criterion here.
• A byte-bounded frame queue, a 1013 disconnect, and writes that cannot starve liveness (relay: the outbound frame queue is unbounded, so a slow client becomes relay memory #952). FrameSink counts admitted bytes (compare_exchange on a shared counter, so concurrent per-message tasks cannot lose an increment), admits a frame only while it fits or the queue is empty, and on overflow drops the frame, sets a sticky flag and logs once per racing sender. run_connection reads the flag at the top of every iteration and closes the client with 1013 and slow consumer: outbound frame queue full, then falls through to the existing cleanup. Every write the connection task performs — queued frame, pong, ping, both error closes, each drain CLOSED frame and the drain close — is wrapped in tokio::time::timeout(write_timeout, ..), so no select arm can stay pending longer than that and the liveness tick is delayed by at most 30 s instead of unboundedly.
• Two RelayConfig knobs, frame_queue_bytes (8 MiB) and write_timeout (30 s), with the default timeout shorter than the 120 s liveness interval. They are not settings: the capture has no counterpart for either, so a settings key would mirror nothing — resources/default-settings.yaml and nostream-config are untouched.
• docs/rewrite/decisions/0005-connection-concurrency.md records the three decisions, the alternatives rejected and why (including the pause-the-producer self-deadlock: dispatch_live sends from the loop that drains, so a permit wait there waits on the task that would release it), and states the port's CLOSE-aborts-a-read semantics as out of scope for that record.

Evidence

Oracle cargo test -p nostream-relay --all-features: 133 passed, 0 failed at base 129 — lib 7→8, connection 14→15, handlers 49→50, transport 13→14, everything else unchanged.

Each deciding test was written and run against the unmodified sources first:

criterion	at base	after
#950 read ownership	Err(RecvError(())) on the replacement's channel, in_flight=false	passes
#951 CLOSE during the mint	three frames where one is required (invite EVENT, stored EVENT, EOSE)	one frame, no EOSE
#952 the byte bound	compile error (E0061, E0599): no bound exists	passes, including a lost-increment guard
#952 the disconnect policy	no 0x8 frame in 9.96 s, last opcode 1	0x8, code 1013, the fixed reason

The reworked disconnect test is falsifiable and was shown red for each of two mutations (bound raised out of reach; stored events removed) before being restored and shown green. cargo test --workspace --all-features reports 0 failed on every target, cargo fmt --all --check and both CLAUDE.md clippy passes exit 0, and no test was renamed, #[ignore]d or relaxed (ts_case_parity 4, wire_parity 5, no fixtures touched).

Liveness while a write is blocked is the one criterion no command settles here: a loopback socket cannot be forced to block deterministically. It is recorded as judged on the diff (every write site wrapped, 30 s against the 120 s interval), not as measured.

Follow-ups filed separately

• rewrite: record the CLOSE-aborts-read decision the port already makes #957 — the port aborts a stored read on a plain CLOSE and the capture does not; no decision record says so. Out of scope here, by ruling.
• relay: inbound frames dispatch out of order, so a CLOSE can overtake its REQ #958 — a CLOSE whose handler task is scheduled ahead of its REQ still aborts nothing (each client message is spawned independently and abort_read leaves no tombstone). Pre-existing; the fix above narrows the window but does not close it, and the ADR states that as a precondition rather than claiming it.
• relay: the 8 MiB frame bound is per socket and sockets are unlimited #959, relay: a float-spelled timestamp is rejected, which the capture accepts #960, relay: extra frame elements are ignored where the capture rejects them #961 — the per-socket bound is not a process ceiling and Relay::serve accepts without a cap; two message-decoding divergences the static analyser found in nostream-core.

[changeset-bot]

🦋 Changeset detected

Latest commit: 11177d3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 0 packages

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[greptile-apps]

Confidence Score: 4/5

[Medium impact] The changes affect connection behavior across the Rust relay's client sessions.

The PR appears safe to merge, with a non-blocking timing problem in the new cancellation test.

Findings

1.  Cancellation test can fail randomly ▶

Summary

The PR keeps read cleanup from removing a replacement read, registers cancellation before invite creation, and adds a byte limit and write timeouts to outbound frames.

• Replacement reads keep their own cancellation entries.
• A close during invite minting stops the stored read.
• Slow clients get a bounded frame queue and timed writes.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[REQ handler] --> B[Register read with generation]
  B --> C[Mint invite if requested]
  C --> D{Read cancelled?}
  D -->|Yes| E[End without stored events or EOSE]
  D -->|No| F[Stream stored events]
  F --> G[FrameSink checks byte limit]
  G -->|Fits| H[Queue frame]
  G -->|Full| I[Set overflow flag]
  H --> J[Write with timeout]
  J --> K[Release counted bytes]
  I --> L[Connection closes with 1013]

Loading

_{Reviews (1) · Last reviewed commit: "docs(relay): state the cut's true bounda..." · Reviewed by Greptile}

[greptile-apps]

Cancellation test can fail randomly

The test waits 50 ms after sending CLOSE, but nothing confirms that the server has handled it before gate.add_permits(1) releases the mint. If a busy runner delays the CLOSE handler, the mint can resume first and send stored events or EOSE. The test then fails even when the fix is correct. Wait for a server-side signal that abort_read has run before releasing the gate.