fix tip formatting - #4
Merged
Merged
Conversation
shreyav
marked this pull request as ready for review
October 10, 2025 23:37
shreyav
force-pushed
the
10-10-fix_tip_formatting
branch
from
October 10, 2025 23:39
bdb5517 to
7a6813b
Compare
jklein24
approved these changes
Oct 10, 2025
Draft
pengying
added a commit
that referenced
this pull request
Jul 27, 2026
…ents (#742) ## Summary Fan-out to multiple clients already worked; **replay on reconnect didn't**. This adds it. The demo drives one hardcoded customer, so every connected panel is watching the same account and should see every delivery. There is no per-client filtering — `pushEvent` walks all subscribers — and that part is verified below. The gap was that a client only ever saw events that arrived *while it was connected*, so an `EventSource` auto-reconnect after a network blip silently dropped whatever landed in the gap. ### What changed - Every frame now carries an `id:` (a per-process sequence number). `EventSource` echoes the last one back as `Last-Event-ID` when it reconnects, and the stream replays events after that point. - **A fresh client replays nothing.** It presents no id, and replaying history would render old deliveries as if they had just arrived — the panel timestamps entries on receipt. - **Subscribe happens before replay**, so a delivery landing mid-replay queues behind it rather than slipping between the two steps. - The connect handshake comment reports `clients=N`. `EventSource` ignores comment frames; it shows up in `curl -N .../api/webhooks/stream` when you want to confirm every panel is attached. ### Verified Three concurrent clients, one delivery each time, with a disconnect and a reconnect in the middle: ``` A handshake: : connected clients=1 B: clients=2 C: clients=3 deliver #1 → A [1] | B [1] | C [1] ← every client, no filtering B disconnects; #2 and #3 land while it is away A [1,2,3] | C [1,2,3] ← survivors unaffected B reconnects with Last-Event-ID: 1 B replayed: [2,3] ← exactly what it missed D connects fresh: [] ← no invented history deliver #4 → A [1,2,3,4] | C [1,2,3,4] | B [2,3,4] | D [4] ``` Signature verification is untouched; the replay path only re-sends events that already passed it. ### Limits (documented in the module) - The bus is **per server process**. With multiple replicas, only the instance that received a delivery can fan it out to its own clients — a shared channel (Redis pub/sub) would be needed. - The ring buffer holds the **last 50** events, so a client away longer than that loses the overflow. Stacked on #730. ### Update: catch-up must not duplicate either Self-review caught a flaw in the first push. Subscribe-then-replay avoids gaps but **double-sends**: an event landing between `subscribe` and the replay loop goes out live *and* again from the backlog. Replay-then-subscribe has the opposite bug (it loses that event). The stream now subscribes immediately but **holds** live events, replays the backlog, then flushes the held ones the replay didn't cover. Verified by reconnecting from `Last-Event-ID: 1` while deliveries stream in: ``` client saw: 2,3,4,5,6 duplicates: none gaps : none (2..6 as expected) ``` Also added `webhookEvents.test.ts` covering the bus contract: fan-out to every subscriber, survival of a subscriber that throws, the `seq`/`eventsSince` replay windows, and the ring dropping the overflow a long-gone client can no longer catch up on. **69 tests** total (was 60). ### Update: the stream was readable by any site Second self-review finding, and a more serious one. `/api/webhooks/stream` is unauthenticated and carries whatever Grid delivered — transaction ids, amounts, and `counterpartyInformation`, which per Grid's schema can include a counterparty's **name, birth date and nationality**. That is the same exposure `GET /api/webhooks/events` was deleted for earlier in this stack; the SSE replacement inherited it. An `EventSource` opened by another site is now refused (`Sec-Fetch-Site: cross-site` → 403), so a page on someone else's origin can't attach to a panel running on your machine: ``` same-origin (our own panel) → 200 no header (curl / scripts) → 200 cross-site (another website) → 403 ``` **This is not access control**, and the code comment says so: anyone who can reach the host can still read the stream, which behind an ngrok tunnel means anyone with the URL. Gating it properly — a session cookie, or not exposing it past localhost — belongs on the pre-prod list next to proxy customer-scoping. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Pjqydq34z8DBK1kBrioXK2
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.