Repository navigation
docs(rewrite): record that a CLOSE aborts the read it closes (#957) - #988
Conversation
🦋 Changeset detectedLatest commit: 2548900 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen 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 |
…957) Three things the job closed without, all named in the brief it was given: - Status said 'accepted (job 015, Phase 2)'. It is the maintainer's on merge, and the job has no Phase 2. - The quotation of 0005:212-215 showed the wording this branch had already rewritten, so the citation resolved to different text. Marked as the pre-amendment wording. - No changeset: CONTRIBUTING.md:364 fails changeset-check without one, and :378-387 documents the empty changeset for docs-only work. An empty changeset: no package version moves for a decision record.
 [Low impact] The PR appears safe to merge; the only finding is a non-blocking correction to test citations. Findings
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| The socket test gates the store read before any record (`:1029`; the gate is | ||
| `crates/nostream-relay/src/testing.rs:203-207`) and asserts no message within two seconds | ||
| (`:1041`). The registration assertion after `handlers::begin` returns `None` for a `CLOSE` |
There was a problem hiding this comment.
:1029 is an assertion in the preceding test, and :1041 starts the server. Neither shows the gate or silence check described here, so readers following the citations miss the supporting evidence. Update them to handlers.rs:1038 and handlers.rs:1050, and update the earlier test range to 1034-1059.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Brings in #987, #988 and #990, which landed after this branch was cut. Three conflicts, all resolved toward this branch's side because this branch is the one that changes what they describe: - `relay.rs` doc comments: main still calls the two bounds constants that belong in `Settings`, which is the state #990 left. This branch makes them operator settings read by `RelayConfig::from_settings`, so its wording is the one that matches the code. - `0008-frame-memory-budget.md`: main carries #990's version, where the settings key is future work (#989) and a zero is unvalidated. This branch *is* that work, so its side carries the resolution, the `What 0 means` section and the citation re-anchors. - `0005-connection-concurrency.md`: identical prose, wrapped differently. Every citation in both ADRs was re-verified against the merged tree with the factory's `citation-meaning` tool: 68 in 0008 and 8 in 0005, all resolving, exit 0. The anchors differ from main's by design — this branch adds code, so line numbers move. Verified: `cargo clippy --all-targets -p nostream-relay -- -D warnings` clean; `cargo test -p nostream-relay` 242 tests passing across 10 targets.
The port aborts an in-flight stored read when a
CLOSEarrives; the TypeScript capture does not. Nothing recorded that, and the repository's rule is that the capture wins unless a decision record says otherwise — so until now the port held an unrecorded deviation, and anyone reading the two side by side would have read it as a defect.This PR writes the missing record:
docs/rewrite/decisions/0007-close-aborts-read.md, and amends one pointer paragraph in0005that deferred the question to this issue.Closes #957.
The decision
CLOSEaborts. The record states it, with the two implementations' behaviour, the argument, the price, and what would reopen it.CLOSEis a prefix on the connection loop (handlers/mod.rs:243-247), the handler isabort_readthenunsubscribe(unsubscribe.rs:14-16), the abort lands on the read's next poll (subscribe.rs:243-248), and the client sees no frame — noEVENT, noEOSE.UnsubscribeMessageHandleremits onlyUnsubscribe(unsubscribe-message-handler.ts:9-11), the adapter'sonUnsubscribedonly deletes the map entry (web-socket-adapter.ts:107-110), and the read's pipeline has no subscription-liveness filter — so the remainingEVENTs and theEOSEarrive for an id the client has closed. Its abort machinery exists but is unreachable fromCLOSE:abortInFlightSubscriptionReadshas one caller,endSubscription, reached only from the rate-limitedREQpath (:303,:312-320).CLOSEis "used to stop previous subscriptions" (01.md:119) and a relaySHOULDstop sending matching events once one arrives (:137), quoted atnostr-protocol/nips@0b5532c0.The price, stated in the record
Byte-parity with the capture for a
CLOSEthat lands mid-read: the trailing stored frames and theEOSE. The differential oracle cannot see the difference — its store double streams unpaced, a{none: true}step waits 250 ms and sweeps, and the session format has no field to pace one — so the behaviour is pinned by a gated socket test (tests/handlers.rs:1025-1050), not by the oracle. An abort is also not a recall: frames already in the byte-boundedFrameSinkmay reach the client, and the record marks that race unmeasured rather than argued.What is not decided here
The capture's unguarded invite emit is already a recorded choice (
0005:136-149); this record cites it in one sentence and does not re-decide or supersede it. No code changes, no fixture work.Scope of the diff
Two files, documentation only:
docs/rewrite/decisions/0007-close-aborts-read.md, new, in0006's shape.docs/rewrite/decisions/0005-connection-concurrency.md: the paragraph that deferred this question to rewrite: record the CLOSE-aborts-read decision the port already makes #957 now points at0007. It also carries one unrelated path correction at:42, wheretests/connection.rswas ambiguous againstsrc/connection.rs— the citation checker rejects an ambiguous basename, and the record was being edited anyway.Verification
docs/**runs no Rust job in CI, so the checks are citation and reading:citation-oracle.tsexits 0 on both files (refs 31, shas 1on 0007;refs 6on 0005), with the pinned NIPs checkout supplied as an extra root, and the two NIP-01 sentences matched verbatim against0b5532c0.git merge-treeontorewrite/mainis clean.The static reviewer's one finding — a paragraph calling the
FrameSinkan unbounded queue when it is byte-bounded with a drop-and-close policy — is corrected, as is its successor: at overflow the client is closed with1013and the queue is discarded, not drained (relay.rs:330-333,:427-448, against the drain at:336-351).Note
Merging is what accepts this record — the status line says
acceptedbecause the port has run this way since it existed, and a maintainer's merge is the act that makes the deviation recorded rather than tolerated.