fix(sync): scan events keep instance-wide fan-out as the sanctioned BroadcastToAll
Verifying the per-user chokepoint end to end surfaced a UI regression it would have caused: web/src/header.ts runs the sitewide scan indicator on scan_progress/scan_complete events in every browser, and routing those to the triggering identity alone froze the indicator for everyone else at its page-load state (active-summary stays the load-time fallback, but live movement died). The source's own comment declares the indicator sitewide by design, and scan payloads carry no user data — job ids, file counts, progress percentages only. Resolution honors both truths: BroadcastToAll is re-added as the ONE sanctioned instance-wide fan-out, its doc comment making the decision explicit (no user-derived data; anything carrying books, notes, or annotations belongs in BroadcastToUser), and the two scan emitters use it. TestBroadcastToAllReachesEveryone pins the exception alongside TestBroadcastIsPerUser — audiences are now named decisions the compiler and the tests both enforce.
This commit is contained in:
1 parent
a431dfa10b
commit
dd963993eb
4 files changed
+77
-13
No files matched your search
@@ -75,13 +75,14 @@ nothing to implement. A connection that stops answering is removed.
|
|||||||
|
|
||||||
## Fan-out and backpressure — read this before building a client
|
## Fan-out and backpressure — read this before building a client
|
||||||
|
|
||||||
- **Every event is delivered only to the acting user's connections.**
|
- **Every event carries an explicit audience.** Progress, annotation,
|
||||||
Progress, annotation, and conflict events go to the user whose sync
|
conflict, and collection events are delivered only to the acting
|
||||||
activity produced them; scan events go to the identity that triggered
|
user's connections — the ConnectionManager has no accidental
|
||||||
the job; collection events to the collection's owner. The
|
fan-out, and the compiler forces every emitter to name its audience
|
||||||
ConnectionManager has no untargeted broadcast — the compiler enforces
|
(2026-11-02, after cross-user delivery was found and fixed). The one
|
||||||
an audience on every emitter (2026-11-02, after cross-user delivery
|
sanctioned instance-wide fan-out is **scan events**
|
||||||
was found and fixed).
|
(`BroadcastToAll`): they carry job ids and counts, never user data,
|
||||||
|
and the sitewide scan indicator in every browser runs on them.
|
||||||
- Every connection has a bounded send queue. **If a client falls behind,
|
- Every connection has a bounded send queue. **If a client falls behind,
|
||||||
messages for that connection are skipped, not queued.** Clients must
|
messages for that connection are skipped, not queued.** Clients must
|
||||||
therefore treat the socket as a *hint* — on reconnect (or any gap),
|
therefore treat the socket as a *hint* — on reconnect (or any gap),
|
||||||
|
|||||||
@@ -402,15 +402,18 @@ func (w *Worker) processJob(job *Job) {
|
|||||||
Errors: errors,
|
Errors: errors,
|
||||||
}
|
}
|
||||||
|
|
||||||
if job.Type == JobTypeScan && w.connManager != nil && job.UserID != "" {
|
if job.Type == JobTypeScan && w.connManager != nil {
|
||||||
// Cancelled scans announce scan_cancelled so clients reset
|
// Cancelled scans announce scan_cancelled so clients reset
|
||||||
// their scanning indicators; a "scan_complete" would read as
|
// their scanning indicators; a "scan_complete" would read as
|
||||||
// a finished scan.
|
// a finished scan.
|
||||||
|
// Scan events are instance activity, not user data (job ids and
|
||||||
|
// counts only) — they fan out to every client so the sitewide
|
||||||
|
// scan indicator works for everyone. See BroadcastToAll.
|
||||||
msgType := wsync.MessageTypeScanComplete
|
msgType := wsync.MessageTypeScanComplete
|
||||||
if status == JobStatusCancelled {
|
if status == JobStatusCancelled {
|
||||||
msgType = wsync.MessageTypeScanCancelled
|
msgType = wsync.MessageTypeScanCancelled
|
||||||
}
|
}
|
||||||
w.connManager.BroadcastToUser(job.UserID, wsync.BroadcastMessage{
|
w.connManager.BroadcastToAll(wsync.BroadcastMessage{
|
||||||
Type: msgType,
|
Type: msgType,
|
||||||
Data: map[string]interface{}{
|
Data: map[string]interface{}{
|
||||||
"job_id": job.ID,
|
"job_id": job.ID,
|
||||||
@@ -464,8 +467,10 @@ func (w *Worker) processScanJob(job *Job) (interface{}, error) {
|
|||||||
result.NewItems = newItems
|
result.NewItems = newItems
|
||||||
result.Errors = errors
|
result.Errors = errors
|
||||||
}
|
}
|
||||||
// Broadcast via WebSocket to user
|
// Broadcast scan progress to every connected client — instance
|
||||||
if w.connManager != nil && job.UserID != "" {
|
// activity, no user data (see BroadcastToAll). The sitewide
|
||||||
|
// header indicator runs on these events.
|
||||||
|
if w.connManager != nil {
|
||||||
msg := wsync.BroadcastMessage{
|
msg := wsync.BroadcastMessage{
|
||||||
Type: wsync.MessageTypeScanProgress,
|
Type: wsync.MessageTypeScanProgress,
|
||||||
Data: map[string]interface{}{
|
Data: map[string]interface{}{
|
||||||
@@ -477,7 +482,7 @@ func (w *Worker) processScanJob(job *Job) (interface{}, error) {
|
|||||||
},
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
w.connManager.BroadcastToUser(job.UserID, msg)
|
w.connManager.BroadcastToAll(msg)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -71,6 +71,28 @@ func NewConnectionManager() *ConnectionManager {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// BroadcastToAll delivers an event to every connected client on the
|
||||||
|
// instance. This is the ONE sanctioned instance-wide fan-out, and using
|
||||||
|
// it is a decision: the payload must contain no user-derived data. Its
|
||||||
|
// only caller is the scan-event emitter — scan activity is instance
|
||||||
|
// activity (job ids, file counts, progress percentages), and the
|
||||||
|
// sitewide scan indicator in web/src/header.ts is built on receiving it
|
||||||
|
// in every browser. Anything that carries a user's books, notes, or
|
||||||
|
// annotations belongs in BroadcastToUser.
|
||||||
|
func (m *ConnectionManager) BroadcastToAll(msg BroadcastMessage) {
|
||||||
|
m.mu.RLock()
|
||||||
|
defer m.mu.RUnlock()
|
||||||
|
|
||||||
|
for _, conn := range m.connections {
|
||||||
|
select {
|
||||||
|
case conn.Send <- msg:
|
||||||
|
default:
|
||||||
|
// Channel full, skip this connection
|
||||||
|
log.Printf("WebSocket: Channel full for %s, skipping broadcast", conn.DeviceName)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// BroadcastToUser sends a message to all connections for a specific user
|
// BroadcastToUser sends a message to all connections for a specific user
|
||||||
func (m *ConnectionManager) BroadcastToUser(userID string, msg BroadcastMessage) {
|
func (m *ConnectionManager) BroadcastToUser(userID string, msg BroadcastMessage) {
|
||||||
m.mu.RLock()
|
m.mu.RLock()
|
||||||
|
|||||||
@@ -68,8 +68,44 @@ func TestBroadcastIsPerUser(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// ...and Bob received nothing. Any message here is a cross-user leak.
|
// ...and Bob received nothing user-derived from Alice's sync
|
||||||
|
// activity. Scan events are the one sanctioned instance-wide fan-out
|
||||||
|
// (BroadcastToAll — job ids and counts, no user data); everything
|
||||||
|
// carrying a user's books or annotations stops at that user's edge.
|
||||||
if leaked := drain(b1); len(leaked) != 0 {
|
if leaked := drain(b1); len(leaked) != 0 {
|
||||||
t.Fatalf("bob's client received %d message(s) from alice's sync activity — cross-user leak", len(leaked))
|
t.Fatalf("bob's client received %d message(s) from alice's sync activity — cross-user leak", len(leaked))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestBroadcastToAllReachesEveryone pins the sanctioned exception: scan
|
||||||
|
// activity is instance-wide (the sitewide header indicator runs on it),
|
||||||
|
// while everything user-derived stays per-user (TestBroadcastIsPerUser).
|
||||||
|
func TestBroadcastToAllReachesEveryone(t *testing.T) {
|
||||||
|
cm := NewConnectionManager()
|
||||||
|
|
||||||
|
var conns []*DeviceConnection
|
||||||
|
for _, user := range []string{"alice", "bob", "carol"} {
|
||||||
|
conn := &DeviceConnection{
|
||||||
|
ID: uuid.NewString(),
|
||||||
|
UserID: user,
|
||||||
|
DeviceType: "test",
|
||||||
|
DeviceName: user + "-device",
|
||||||
|
Send: make(chan BroadcastMessage, 4),
|
||||||
|
}
|
||||||
|
cm.AddConnection(conn)
|
||||||
|
conns = append(conns, conn)
|
||||||
|
}
|
||||||
|
|
||||||
|
cm.BroadcastToAll(BroadcastMessage{Type: MessageTypeScanProgress})
|
||||||
|
|
||||||
|
for _, conn := range conns {
|
||||||
|
select {
|
||||||
|
case msg := <-conn.Send:
|
||||||
|
if msg.Type != MessageTypeScanProgress {
|
||||||
|
t.Errorf("%s: got %q, want scan_progress", conn.DeviceName, msg.Type)
|
||||||
|
}
|
||||||
|
default:
|
||||||
|
t.Errorf("%s: received nothing from BroadcastToAll", conn.DeviceName)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in new issue
Block a user