Skip to content

docs(rewrite): record that a CLOSE aborts the read it closes (#957) - #988

Open
chappie-daemon wants to merge 7 commits into
rewrite/mainfrom
factory/015-close-aborts-read-adr
Open

chappie-daemon wants to merge 7 commits into
rewrite/mainfrom
factory/015-close-aborts-read-adr

Conversation

@chappie-daemon

Copy link
Copy Markdown
Collaborator

The port aborts an in-flight stored read when a CLOSE arrives; 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 in 0005 that deferred the question to this issue.

Closes #957.

The decision

CLOSE aborts. The record states it, with the two implementations' behaviour, the argument, the price, and what would reopen it.

  • The port: CLOSE is a prefix on the connection loop (handlers/mod.rs:243-247), the handler is abort_read then unsubscribe (unsubscribe.rs:14-16), the abort lands on the read's next poll (subscribe.rs:243-248), and the client sees no frame — no EVENT, no EOSE.
  • The capture: UnsubscribeMessageHandler emits only Unsubscribe (unsubscribe-message-handler.ts:9-11), the adapter's onUnsubscribed only deletes the map entry (web-socket-adapter.ts:107-110), and the read's pipeline has no subscription-liveness filter — so the remaining EVENTs and the EOSE arrive for an id the client has closed. Its abort machinery exists but is unreachable from CLOSE: abortInFlightSubscriptionReads has one caller, endSubscription, reached only from the rate-limited REQ path (:303, :312-320).
  • NIP-01: CLOSE is "used to stop previous subscriptions" (01.md:119) and a relay SHOULD stop sending matching events once one arrives (:137), quoted at nostr-protocol/nips@0b5532c0.

The price, stated in the record

Byte-parity with the capture for a CLOSE that lands mid-read: the trailing stored frames and the EOSE. 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-bounded FrameSink may 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, in 0006'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 at 0007. It also carries one unrelated path correction at :42, where tests/connection.rs was ambiguous against src/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.ts exits 0 on both files (refs 31, shas 1 on 0007; refs 6 on 0005), with the pinned NIPs checkout supplied as an extra root, and the two NIP-01 sentences matched verbatim against 0b5532c0. git merge-tree onto rewrite/main is clean.

The static reviewer's one finding — a paragraph calling the FrameSink an 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 with 1013 and 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 accepted because 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.

@changeset-bot

changeset-bot Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 40715ea

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

…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.
@greptile-apps

greptile-apps Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Low impact] This documentation-only PR appears safe to merge.

Summary

Records the existing Rust choice to abort a stored read when its client sends CLOSE, despite the TypeScript capture continuing that read.

  • A new decision record explains why the port stops reads on CLOSE.

chappie-daemon explicitly acknowledges that already queued frames may still arrive after CLOSE. The record leaves that behavior unchanged and describes the race as unmeasured.

Reviews (1) · Last reviewed commit: "docs(rewrite): add the empty changeset A..." · Reviewed by Greptile

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant