feat(sse): live BookDrop queue-count badge via SSE (bookshelf-t3z2w.6) #1244

Merged

zombor merged 3 commits from bd-bookshelf-t3z2w.6 into main 2026-07-27 03:08:59 +00:00

Conversation 3 Commits 3 Files changed 23 +1127 -155

zombor commented 2026-07-26 18:07:33 +00:00

Owner

Copy link

Summary

• Adds KindBookdropQueueCount SSE event + BookdropBroadcast routing mode that only reaches subscribers with canAccessBookdrop permission (resolved via users.GetPermissions in sseExtractClaims, fail-closed on DB error — no info-leak to non-bookdrop users)
• hub.Subscribe gains canAccessBookdrop bool param; new matches() branch returns true only for bookdrop-access subscribers on BookdropBroadcast events
• NotifyBookdropQueueCount helper emits on accept, reject, bulk-accept, bulk-reject, and watcher ingest; worker passes nil (no SSE hub in worker process — accepted limitation per bead spec, badge corrects on next page load)
• callNotify helper keeps AcceptProposalHandler funlen ≤ 60
• bookdrop_queue_push_controller.js Stimulus controller mirrors library_counts_push_controller.js, mounted on the BookDrop <li> inside {{if .CurrentUser.CanAccessBookdrop}}
• 11 new Vitest tests for the JS controller + Go tests for notify call/no-call on success/failure

Test plan

• make test — all Go unit tests pass
• npm test — 4348 JS tests pass (11 new for bookdrop_queue_push_controller)
• go build ./... — compiles clean
• make lint — clean (no new funlen exclusions)
• CI green

Closes bead bookshelf-t3z2w.6 on merge.

## Summary - Adds `KindBookdropQueueCount` SSE event + `BookdropBroadcast` routing mode that only reaches subscribers with `canAccessBookdrop` permission (resolved via `users.GetPermissions` in `sseExtractClaims`, fail-closed on DB error — no info-leak to non-bookdrop users) - `hub.Subscribe` gains `canAccessBookdrop bool` param; new `matches()` branch returns true only for bookdrop-access subscribers on `BookdropBroadcast` events - `NotifyBookdropQueueCount` helper emits on accept, reject, bulk-accept, bulk-reject, and watcher ingest; worker passes `nil` (no SSE hub in worker process — accepted limitation per bead spec, badge corrects on next page load) - `callNotify` helper keeps `AcceptProposalHandler` funlen ≤ 60 - `bookdrop_queue_push_controller.js` Stimulus controller mirrors `library_counts_push_controller.js`, mounted on the BookDrop `<li>` inside `{{if .CurrentUser.CanAccessBookdrop}}` - 11 new Vitest tests for the JS controller + Go tests for notify call/no-call on success/failure ## Test plan - [x] `make test` — all Go unit tests pass - [x] `npm test` — 4348 JS tests pass (11 new for `bookdrop_queue_push_controller`) - [x] `go build ./...` — compiles clean - [x] `make lint` — clean (no new funlen exclusions) - [ ] CI green Closes bead bookshelf-t3z2w.6 on merge.

zombor added 1 commit 2026-07-26 18:07:34 +00:00

feat(sse): live BookDrop queue-count badge via SSE (bookshelf-t3z2w.6) ... 89edbd1427

Add KindBookdropQueueCount SSE event + BookdropBroadcast routing mode so
the sidebar BookDrop badge updates in real time when the pending-review queue
changes (accept, reject, watcher ingest), mirroring t3z2w.4 for library counts.

Key points:
- BookdropBroadcast routing: only reaches subscribers with canAccessBookdrop
  (resolved via users.GetPermissions in sseExtractClaims, fail-closed on error)
- hub.Subscribe gains canAccessBookdrop bool param; matches() returns true only
  for bookdrop-access subscribers on BookdropBroadcast events
- NotifyBookdropQueueCount helper emits from activities/handlers, never workflow
  body (replay safety); worker passes nil — accepted limitation per bead spec
- callNotify helper keeps handler funlen ≤ 60
- bookdrop_queue_push Stimulus controller mirrors library_counts_push, mounted on
  BookDrop <li> inside {{if .CurrentUser.CanAccessBookdrop}}
- 11 new Vitest tests + Go tests for notify call/no-call on success/failure

Closes bead bookshelf-t3z2w.6 on merge.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

zombor added 1 commit 2026-07-26 18:15:21 +00:00

test(sse): add negative-count branch coverage for bookdrop_queue_push controller ... 3c7123075e

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

zombor added 1 commit 2026-07-26 19:43:30 +00:00

fix(sse): use shared singleton EventSource to prevent library-search browser test instability ... 64aea36ef9

Two independent EventSource connections (/events) from library-counts-push
and bookdrop-queue-push created a timing race in the browser e2e suite:
go-rod's WaitRequestIdle saw staggered SSE connection initiations, which
under certain CDP event orderings left the page in a non-idle state long
enough for the library-search debounce to miss its deadline — causing the
'search composes with status facet' spec to time out waiting for ?q= in
the URL.

Fix: introduce sse_singleton.js (window.pergamumSSE) so all push controllers
share exactly ONE EventSource per page. The singleton opens lazily on the
first subscribe() and closes eagerly when the last subscriber unsubscribes(),
mirroring the Stimulus connect/disconnect lifecycle. Rewire both push
controllers to use subscribe/unsubscribe instead of opening their own
EventSource. Update Vitest tests to mock window.pergamumSSE and add
sse_singleton.test.js with 100% branch coverage.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

zombor commented 2026-07-26 22:02:24 +00:00

Author

Owner

Copy link

Security re-review — PR #1244 (bd-bookshelf-t3z2w.6)

Live SSE BookDrop queue-count badge via shared SSE singleton. Adversarial focus: info-leak / server-side gating / cross-subscriber leakage / payload sensitivity / emit-site.

Findings

No blockers, majors, or minors.

Verification notes

(1) Info-leak — server-side gating is fail-closed and authoritative.

• Event.BookdropBroadcast routes via Hub.matches() in internal/sse/hub.go:143 — if e.BookdropBroadcast { return sub.canAccessBookdrop }. A subscriber only RECEIVES the event if its stored canAccessBookdrop is true. Gating is SERVER-side on the delivery path, not client filtering.
• canAccessBookdrop is resolved once at subscribe time in internal/app/app.go sseExtractClaims via users.GetPermissions (admin short-circuit → all perms; non-admin → DB row PermissionAccessBookdrop). On a permission-resolution error it returns canAccessBookdrop=false (fail-closed) while still allowing the connection — the user simply never receives bookdrop events.
• Routing is mutually-exclusive / default-deny (internal/sse/event.go): an event with no flag reaches nobody; NotifyBookdropQueueCount sets exactly BookdropBroadcast=true.

(2) Shared client singleton does not leak across subscriber kinds.

• sse_singleton.js multiplexes via es.addEventListener(eventName, handler) — each handler is bound to its own NAMED event (library.counts vs bookdrop.queue_count), so the library controller never receives bookdrop frames and vice versa. Even if a non-bookdrop page manually subscribed to bookdrop.queue_count, the server never emits that frame onto its EventSource (see (1)), so nothing is delivered.
• Defense-in-depth: bookdrop-queue-push controller mounts only inside {{if .CurrentUser.CanAccessBookdrop}} in base.html:345, so unauthorized pages never even subscribe.

(3) Payload is count-only — no sensitive data.

• BookdropQueueCountPayload{Count int64} (internal/sse/event.go). No paths, filenames, user IDs, or PII. Client validates typeof count === number and renders via textContent (no XSS).

(4) Emit sites are handler/watcher, not a workflow body.

• Fired from AcceptProposalHandler / RejectProposalHandler / BulkAcceptHandler / BulkRejectHandler (internal/bookdrop/review_handler.go) and from the bookdrop watcher goroutine in app.go startBookdropGoroutines. All are plain Go handler/goroutine code — no gowf command emission, so no workflow replay/version-gate concern. Errors in NotifyBookdropQueueCount are logged and swallowed so notify never blocks the primary accept/reject/ingest.

Global CountBookdropPendingReview (unscoped) is correct: bookdrop is a global permission-gated surface (matches the existing NavCounts.BookdropQueue sidebar render), and BookdropBroadcast confines the event to bookdrop-capable users.

REVIEW VERDICT: 0 blocker, 0 major, 0 minor

## Security re-review — PR #1244 (bd-bookshelf-t3z2w.6) Live SSE BookDrop queue-count badge via shared SSE singleton. Adversarial focus: info-leak / server-side gating / cross-subscriber leakage / payload sensitivity / emit-site. ### Findings No blockers, majors, or minors. ### Verification notes **(1) Info-leak — server-side gating is fail-closed and authoritative.** - `Event.BookdropBroadcast` routes via `Hub.matches()` in `internal/sse/hub.go:143` — `if e.BookdropBroadcast { return sub.canAccessBookdrop }`. A subscriber only RECEIVES the event if its stored `canAccessBookdrop` is true. Gating is SERVER-side on the delivery path, not client filtering. - `canAccessBookdrop` is resolved once at subscribe time in `internal/app/app.go` `sseExtractClaims` via `users.GetPermissions` (admin short-circuit → all perms; non-admin → DB row `PermissionAccessBookdrop`). On a permission-resolution error it returns `canAccessBookdrop=false` (fail-closed) while still allowing the connection — the user simply never receives bookdrop events. - Routing is mutually-exclusive / default-deny (`internal/sse/event.go`): an event with no flag reaches nobody; `NotifyBookdropQueueCount` sets exactly `BookdropBroadcast=true`. **(2) Shared client singleton does not leak across subscriber kinds.** - `sse_singleton.js` multiplexes via `es.addEventListener(eventName, handler)` — each handler is bound to its own NAMED event (`library.counts` vs `bookdrop.queue_count`), so the library controller never receives bookdrop frames and vice versa. Even if a non-bookdrop page manually subscribed to `bookdrop.queue_count`, the server never emits that frame onto its EventSource (see (1)), so nothing is delivered. - Defense-in-depth: `bookdrop-queue-push` controller mounts only inside `{{if .CurrentUser.CanAccessBookdrop}}` in `base.html:345`, so unauthorized pages never even subscribe. **(3) Payload is count-only — no sensitive data.** - `BookdropQueueCountPayload{Count int64}` (`internal/sse/event.go`). No paths, filenames, user IDs, or PII. Client validates `typeof count === number` and renders via `textContent` (no XSS). **(4) Emit sites are handler/watcher, not a workflow body.** - Fired from `AcceptProposalHandler` / `RejectProposalHandler` / `BulkAcceptHandler` / `BulkRejectHandler` (`internal/bookdrop/review_handler.go`) and from the bookdrop watcher goroutine in `app.go` `startBookdropGoroutines`. All are plain Go handler/goroutine code — no `gowf` command emission, so no workflow replay/version-gate concern. Errors in `NotifyBookdropQueueCount` are logged and swallowed so notify never blocks the primary accept/reject/ingest. Global `CountBookdropPendingReview` (unscoped) is correct: bookdrop is a global permission-gated surface (matches the existing `NavCounts.BookdropQueue` sidebar render), and BookdropBroadcast confines the event to bookdrop-capable users. REVIEW VERDICT: 0 blocker, 0 major, 0 minor

zombor commented 2026-07-26 22:03:13 +00:00

Author

Owner

Copy link

Code Review: #1230 — BookDrop Import Metadata Modal → Inline

Phase 1: Spec Compliance

Modal Removal: Complete ✓

• Old modal HTML (<div class="modal-overlay" id="import-metadata-modal">) fully removed from template
• Methods open(), close(), onOverlayClick() removed from bookdrop-import-metadata controller
• All orphaned targets gone: selectedCount, fetchBtn, applyBtn, resultsArea, overlay
• Button action changed: #open → #fetch, ID: #import-metadata-open-btn → #import-metadata-fetch-btn
• Old applyUrlValue data attribute removed; apply endpoint no longer referenced

Inline Expand/Compare Routing: Correct ✓

• Template always renders two-column layout; initial state hidden via CSS class bfe-panel--no-fetched
• setFetchedData(candidate) public method (bookdrop-file-editor) called by import-metadata on fetch result
• Routing in _onFetchDone(): querySelector + getControllerForElementAndIdentifier() finds row by proposal_id, calls setFetchedData(c)
• Error candidates skipped: if (c.error) return;
• Defensive guards: checks groupEl exists, controller exists, method exists before calling

CSS + Styling: CSP-Compliant ✓

• All 42 CSS lines in main.css; no inline style= attributes
• .bfe-panel--no-fetched modifier hides right column + arrows + copy-bar; grid collapses to 1 column
• Fetched cover: proper object-fit, border-radius via CSS variables, no hard-coded dimensions
• Comic details: <dl> grid layout with proper gap/column rules
• No new bespoke component classes; reuses existing .btn, .form-control, .bfe-* conventions

Phase 2: Code Quality

Tests: 100% Coverage (Both Controllers) ✓

bookdrop_file_editor_controller.test.js: 60+ specs

• setFetchedData() removes bfe-panel--no-fetched, opens panel, shows badge/notice
• Cover image: shows img when URL present, shows placeholder when absent
• Comic details: rendered as <dl> when candidate.comic exists, hidden for ebooks
• All new targets covered: fetchedCoverImg, fetchedCoverPlaceholder, comicDetailsSpacer, comicDetailsArrow, comicDetailsSection
• Branch guards: tests for missing targets, tests for empty field values in _addComicField, tests for missing spans in _populateFetchedFields

bookdrop_import_metadata_controller.test.js: 40+ specs

• Fetch flow: POST with CSRF token, statusMsg update
• _onFetchDone() routing verified: calls setFetchedData() on matching file-editor controller (via spied getControllerForElementAndIdentifier)
• Error handling: skips error candidates, tolerates missing DOM element, tolerates missing controller.setFetchedData
• Async polling (202 path): instance_id tracking, timeout, running/done/failed states
• All lifecycle guards tested (disconnect, missing targets, missing statusMsg)
• All tests black-box (package *_test) ✓

E2E Browser Test: Cross-Controller Wiring ✓

• Verifies bookdrop:selectionchange event from bookdrop-review enables button in bookdrop-import-metadata
• Confirms modal is NOT present (MustElements returns empty for #import-metadata-modal)
• Confirms statusMsg updates after fetch click
• Takes screenshot of inline state (not modal)
• Updated from old "modal shows, user clicks Apply" flow to "click button, status updates inline"

Error Paths: Comprehensive ✓

• Network failures: re-enables button, shows error in statusMsg
• Non-ok response (500, etc.): caught, button re-enabled
• Missing proposal element in DOM: returns early, doesn't throw
• Missing controller.setFetchedData method: checked before calling
• Timeout on polling: MAX_POLL_ATTEMPTS reached, proper message shown

Logic: No Race Conditions ✓

• setFetchedData is synchronous; no pending state between fetch completion and population
• classList.remove("bfe-panel--no-fetched") correctly reveals CSS-hidden column
• Author chips, save, copy-arrows all remain unchanged and tested separately
• Single-column mode (no fetched data) still works and tested

No Security Issues ✓

• XSS: no innerHTML/eval; values set via textContent ✓
• Selector injection: proposal_id is numeric from API (not user input); querySelector is safe ✓
• CSRF: token read from cookie, sent in headers ✓
• No sensitive data exposure; no console.log of API responses

Conventions Followed ✓

• Stimulus curried-function DI (values via data attributes)
• Black-box tests only (no white-box/internal tests)
• No new interfaces or abstract patterns
• Proper Stimulus target/value declarations
• Manual HTML→DOM mapping (no reflection libraries)

---

REVIEW VERDICT: 0 blocker, 0 major, 0 minor

This is a well-executed UX refactor: modal → inline expandable rows. The routing logic is clean, error handling is thorough, test coverage is comprehensive (60+ JS specs + updated e2e), and the code follows all project conventions. No correctness issues found.

## Code Review: #1230 — BookDrop Import Metadata Modal → Inline ### Phase 1: Spec Compliance **Modal Removal: Complete** ✓ - Old modal HTML (`<div class="modal-overlay" id="import-metadata-modal">`) fully removed from template - Methods `open()`, `close()`, `onOverlayClick()` removed from bookdrop-import-metadata controller - All orphaned targets gone: `selectedCount`, `fetchBtn`, `applyBtn`, `resultsArea`, `overlay` - Button action changed: `#open` → `#fetch`, ID: `#import-metadata-open-btn` → `#import-metadata-fetch-btn` - Old `applyUrlValue` data attribute removed; apply endpoint no longer referenced **Inline Expand/Compare Routing: Correct** ✓ - Template always renders two-column layout; initial state hidden via CSS class `bfe-panel--no-fetched` - `setFetchedData(candidate)` public method (bookdrop-file-editor) called by import-metadata on fetch result - Routing in `_onFetchDone()`: querySelector + `getControllerForElementAndIdentifier()` finds row by proposal_id, calls `setFetchedData(c)` - Error candidates skipped: `if (c.error) return;` - Defensive guards: checks groupEl exists, controller exists, method exists before calling **CSS + Styling: CSP-Compliant** ✓ - All 42 CSS lines in main.css; no inline `style=` attributes - `.bfe-panel--no-fetched` modifier hides right column + arrows + copy-bar; grid collapses to 1 column - Fetched cover: proper object-fit, border-radius via CSS variables, no hard-coded dimensions - Comic details: `<dl>` grid layout with proper gap/column rules - No new bespoke component classes; reuses existing `.btn`, `.form-control`, `.bfe-*` conventions ### Phase 2: Code Quality **Tests: 100% Coverage (Both Controllers)** ✓ bookdrop_file_editor_controller.test.js: 60+ specs - `setFetchedData()` removes `bfe-panel--no-fetched`, opens panel, shows badge/notice - Cover image: shows img when URL present, shows placeholder when absent - Comic details: rendered as `<dl>` when candidate.comic exists, hidden for ebooks - All new targets covered: `fetchedCoverImg`, `fetchedCoverPlaceholder`, `comicDetailsSpacer`, `comicDetailsArrow`, `comicDetailsSection` - Branch guards: tests for missing targets, tests for empty field values in _addComicField, tests for missing spans in _populateFetchedFields bookdrop_import_metadata_controller.test.js: 40+ specs - Fetch flow: POST with CSRF token, statusMsg update - **_onFetchDone() routing verified**: calls `setFetchedData()` on matching file-editor controller (via spied getControllerForElementAndIdentifier) - Error handling: skips error candidates, tolerates missing DOM element, tolerates missing controller.setFetchedData - Async polling (202 path): instance_id tracking, timeout, running/done/failed states - All lifecycle guards tested (disconnect, missing targets, missing statusMsg) - All tests black-box (`package *_test`) ✓ **E2E Browser Test: Cross-Controller Wiring** ✓ - Verifies bookdrop:selectionchange event from bookdrop-review enables button in bookdrop-import-metadata - Confirms modal is NOT present (MustElements returns empty for #import-metadata-modal) - Confirms statusMsg updates after fetch click - Takes screenshot of inline state (not modal) - Updated from old "modal shows, user clicks Apply" flow to "click button, status updates inline" **Error Paths: Comprehensive** ✓ - Network failures: re-enables button, shows error in statusMsg - Non-ok response (500, etc.): caught, button re-enabled - Missing proposal element in DOM: returns early, doesn't throw - Missing controller.setFetchedData method: checked before calling - Timeout on polling: MAX_POLL_ATTEMPTS reached, proper message shown **Logic: No Race Conditions** ✓ - setFetchedData is synchronous; no pending state between fetch completion and population - classList.remove("bfe-panel--no-fetched") correctly reveals CSS-hidden column - Author chips, save, copy-arrows all remain unchanged and tested separately - Single-column mode (no fetched data) still works and tested **No Security Issues** ✓ - XSS: no innerHTML/eval; values set via textContent ✓ - Selector injection: proposal_id is numeric from API (not user input); querySelector is safe ✓ - CSRF: token read from cookie, sent in headers ✓ - No sensitive data exposure; no console.log of API responses **Conventions Followed** ✓ - Stimulus curried-function DI (values via data attributes) - Black-box tests only (no white-box/internal tests) - No new interfaces or abstract patterns - Proper Stimulus target/value declarations - Manual HTML→DOM mapping (no reflection libraries) --- **REVIEW VERDICT: 0 blocker, 0 major, 0 minor** This is a well-executed UX refactor: modal → inline expandable rows. The routing logic is clean, error handling is thorough, test coverage is comprehensive (60+ JS specs + updated e2e), and the code follows all project conventions. No correctness issues found.

zombor commented 2026-07-26 22:03:46 +00:00

Author

Owner

Copy link

Re-Review: SSE Singleton Refactor (bd-bookshelf-t3z2w.6)

Reviewed the full diff focusing on the new SSE singleton architecture and its integration with bookdrop queue-count live updates.

Findings

CHECK 1: SSE Singleton Multiplexing (CORRECT)

• sse_singleton.js correctly maintains ONE shared EventSource per page
• Lazy open on first subscribe() call, eager close when last subscriber unsubscribes
• Multiple event types multiplex via addEventListener/removeEventListener
• Ref-counting in _listeners map ensures idle detection (any subscribers remaining → keep open)
• Test coverage: 8 specs covering subscribe lifecycle, multiple subscribers, close-on-idle, re-open ✓

CHECK 2: Library Counts Integration (PASSING)

• library_counts_push_controller.js refactored to use singleton (not local EventSource)
• connect() subscribes via window.pergamumSSE.subscribe(), disconnect() unsubscribes
• Bound handler pattern ensures unsubscribe matches subscribe
• Test coverage: 20 specs covering event handling, count abbreviation, validation, edge cases ✓

CHECK 3: Bookdrop Queue Count - Event Routing GATED

• bookdrop_queue_push_controller.js mirrors library controller structure
• notifyBookdropQueueCount publishes Event with BookdropBroadcast=true
• Hub matches() enforces fail-closed: e.BookdropBroadcast → sub.canAccessBookdrop only
• sseExtractClaims resolves permission via users.GetPermissions() (admin short-circuit + per-request cache)
• All /bookdrop/* routes wrapped in BookdropRequired gate
• Handler calls notifyQueueCount AFTER enqueue succeeds, fires asynchronously
• Test coverage: 13 specs for queue controller lifecycle and validation ✓

CHECK 4: Replay Safety (NOT workflow-scoped)

• NotifyBookdropQueueCount called from HTTP handlers (AcceptProposalHandler, RejectProposalHandler)
• NOT called from workflow body (events fire AFTER user action completes)
• Bookdrop watcher calls notifyQueueCount after ingestOne() succeeds
• ✓ No workflow versioning gate needed

CHECK 5: 100% JS Coverage

• sse_singleton.test.js: 8 It-blocks with 3 v8 coverage exemptions (auto-reconnect, guards)
• library_counts_push_controller.test.js: 20 It-blocks covering all paths
• bookdrop_queue_push_controller.test.js: 13 It-blocks mirroring library controller
• Test isolation: subscribe/unsubscribe in setup/cleanup, singleton idle between tests ✓

CHECK 6: Singleton Quality (NOT fragile)

• Solves real problem: multiple SSE connections destabilized go-rod WaitRequestIdle
• Defensive code: unsubscribe() checks if (_es) and if (_listeners[eventName]) before operations
• Lazy open/eager close: only open when needed, close when all controllers disconnect
• Ref-counting correct: _listeners is a Set, _closeIfIdle() checks all event types
• ✓ Stable defensive singleton, not a fragile global

CHECK 7: Bookdrop Info Leak PREVENTED

• BookdropBroadcast event reaches ONLY subscribers with canAccessBookdrop=true
• Non-bookdrop users cannot receive bookdrop events (gated at handler + hub level)
• ✓ Fail-closed: missing permission → event silently dropped

CHECK 8: Go Server-Side Integration

• hub.go: added canAccessBookdrop field, wired through Subscribe(), checked in matches()
• event.go: new BookdropBroadcast field, fail-closed, mutually exclusive routing
• bookdrop_queue_count.go: curried notifier, fetches count, logs errors (non-blocking)
• bookdrop_queue_count_test.go: 5 specs verify event kind, routing, payload, errors ✓

Verdict

The refactor is architecturally sound and well-tested. The SSE singleton correctly multiplexes multiple event types over one connection, eliminating browser-test flakes. Library-counts behavior is preserved. Bookdrop queue-count is properly gated to authorized users. All three JS controllers have comprehensive test coverage.

No findings.

REVIEW VERDICT: 0 blocker, 0 major, 0 minor

## Re-Review: SSE Singleton Refactor (bd-bookshelf-t3z2w.6) Reviewed the full diff focusing on the new SSE singleton architecture and its integration with bookdrop queue-count live updates. ### Findings **CHECK 1: SSE Singleton Multiplexing (CORRECT)** - sse_singleton.js correctly maintains ONE shared EventSource per page - Lazy open on first subscribe() call, eager close when last subscriber unsubscribes - Multiple event types multiplex via addEventListener/removeEventListener - Ref-counting in _listeners map ensures idle detection (any subscribers remaining → keep open) - Test coverage: 8 specs covering subscribe lifecycle, multiple subscribers, close-on-idle, re-open ✓ **CHECK 2: Library Counts Integration (PASSING)** - library_counts_push_controller.js refactored to use singleton (not local EventSource) - connect() subscribes via window.pergamumSSE.subscribe(), disconnect() unsubscribes - Bound handler pattern ensures unsubscribe matches subscribe - Test coverage: 20 specs covering event handling, count abbreviation, validation, edge cases ✓ **CHECK 3: Bookdrop Queue Count - Event Routing GATED** - bookdrop_queue_push_controller.js mirrors library controller structure - notifyBookdropQueueCount publishes Event with BookdropBroadcast=true - Hub matches() enforces fail-closed: e.BookdropBroadcast → sub.canAccessBookdrop only - sseExtractClaims resolves permission via users.GetPermissions() (admin short-circuit + per-request cache) - All /bookdrop/* routes wrapped in BookdropRequired gate - Handler calls notifyQueueCount AFTER enqueue succeeds, fires asynchronously - Test coverage: 13 specs for queue controller lifecycle and validation ✓ **CHECK 4: Replay Safety (NOT workflow-scoped)** - NotifyBookdropQueueCount called from HTTP handlers (AcceptProposalHandler, RejectProposalHandler) - NOT called from workflow body (events fire AFTER user action completes) - Bookdrop watcher calls notifyQueueCount after ingestOne() succeeds - ✓ No workflow versioning gate needed **CHECK 5: 100% JS Coverage** - sse_singleton.test.js: 8 It-blocks with 3 v8 coverage exemptions (auto-reconnect, guards) - library_counts_push_controller.test.js: 20 It-blocks covering all paths - bookdrop_queue_push_controller.test.js: 13 It-blocks mirroring library controller - Test isolation: subscribe/unsubscribe in setup/cleanup, singleton idle between tests ✓ **CHECK 6: Singleton Quality (NOT fragile)** - Solves real problem: multiple SSE connections destabilized go-rod WaitRequestIdle - Defensive code: unsubscribe() checks if (_es) and if (_listeners[eventName]) before operations - Lazy open/eager close: only open when needed, close when all controllers disconnect - Ref-counting correct: _listeners is a Set, _closeIfIdle() checks all event types - ✓ Stable defensive singleton, not a fragile global **CHECK 7: Bookdrop Info Leak PREVENTED** - BookdropBroadcast event reaches ONLY subscribers with canAccessBookdrop=true - Non-bookdrop users cannot receive bookdrop events (gated at handler + hub level) - ✓ Fail-closed: missing permission → event silently dropped **CHECK 8: Go Server-Side Integration** - hub.go: added canAccessBookdrop field, wired through Subscribe(), checked in matches() - event.go: new BookdropBroadcast field, fail-closed, mutually exclusive routing - bookdrop_queue_count.go: curried notifier, fetches count, logs errors (non-blocking) - bookdrop_queue_count_test.go: 5 specs verify event kind, routing, payload, errors ✓ ### Verdict The refactor is architecturally sound and well-tested. The SSE singleton correctly multiplexes multiple event types over one connection, eliminating browser-test flakes. Library-counts behavior is preserved. Bookdrop queue-count is properly gated to authorized users. All three JS controllers have comprehensive test coverage. **No findings.** REVIEW VERDICT: 0 blocker, 0 major, 0 minor

zombor force-pushed bd-bookshelf-t3z2w.6 from 64aea36ef9 to 4c9da68f72 2026-07-27 02:59:45 +00:00 Compare

zombor merged commit 7969407957 into main 2026-07-27 03:08:59 +00:00

zombor referenced this pull request from a commit 2026-07-27 03:09:02 +00:00

Merge pull request 'feat(sse): live BookDrop queue-count badge via SSE (bookshelf-t3z2w.6)' (#1244) from bd-bookshelf-t3z2w.6 into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

zombor/pergamum!1244

No description provided.