From f88525091332ee1aebfc5ad9e24219542ab17b49 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 11:17:52 +0100 Subject: [PATCH 1/2] desk-channel: keep where a message is shown when the store does not answer, so its clearing still replaces it On 2026-10-26 09:18 the bus reloading its accounts made the desk's state store answer 'no responders' just as the urgent 'A new build of mesh-controller cannot be delivered' was shown. The desk logged that it could not keep the notification ids and acked the work; the router's clearing at 09:34 then found no record, was shown as a new notification, and the urgent one stayed on the operator's screen for hours. Now what was shown and not kept is held in memory, recalled before the store by edits, reminders, clicks and a notifier restart, kept at the next piece of work, and its work is offered again until the store keeps it, without being shown twice. An edit of a message nothing records is said in the journal. --- modules/desk-channel/cmd/desk-channel/desk.go | 113 ++++++++++++++++-- .../cmd/desk-channel/desk_test.go | 50 +++++++- 2 files changed, 153 insertions(+), 10 deletions(-) diff --git a/modules/desk-channel/cmd/desk-channel/desk.go b/modules/desk-channel/cmd/desk-channel/desk.go index 67ed2746..6d105eeb 100644 --- a/modules/desk-channel/cmd/desk-channel/desk.go +++ b/modules/desk-channel/cmd/desk-channel/desk.go @@ -15,6 +15,11 @@ package main // the desk says in its standing when that happened (`answers-lost-at`), and the router, which alone knows // what is still open and unanswered, says that again under its handle — replacing the notification in place, // so its buttons work again. Nothing resolved, silenced or cancelled comes back. +// - **Where a message is shown is never lost to a store that does not answer.** A message is shown first and +// its notification ids kept after; when the state store does not answer then (the bus reloading its +// accounts, 2026-10-26 09:18: the very outage that raised the condition), the ids stay in memory, are +// recalled from there, and the work is offered again until the store keeps them. Otherwise the clearing +// of that message finds no notification to replace and the urgent one stays on screen for good. // - **Standing**: it can send when machines are named in its settings; which of them answered last is in its // status. A message no machine took is said to the router as failed, and tried again by it. @@ -93,6 +98,9 @@ type Desk struct { pause map[string]pauseSeen // lost is when each machine's notifier last started again: the answers its notifications waited for are lost. lost map[string]time.Time + // unkept is what was shown and not yet kept in the store, because it did not answer: recalled from here + // before the store, and kept at the next piece of work. + unkept map[string]Shown } func (d *Desk) init() { @@ -107,6 +115,9 @@ func (d *Desk) init() { if d.lost == nil { d.lost = map[string]time.Time{} } + if d.unkept == nil { + d.unkept = map[string]Shown{} + } if d.Now == nil { d.Now = time.Now } @@ -138,6 +149,11 @@ func (d *Desk) Take(subject string, body json.RawMessage) error { d.Logf("[desk-channel] work on %s could not be read; taken and dropped", subject) return nil } + if s, ok := d.unkeptOf(m.Handle); ok && sameMessage(s.Message, m) { + // This work offered again because what it showed was not kept: kept now, not shown twice. + return d.keep(m.Handle, s) + } + d.keepUnkept() machines := d.Machines() if m.Secret { // Shown only where the notifier said lately it is not paused (the review of 2026-10-09): one held back @@ -167,12 +183,16 @@ func (d *Desk) Take(subject string, body json.RawMessage) error { switch { case edit: var err error - if before, err = d.Store.Get(m.Handle); err != nil { + if before, err = d.recall(m.Handle); err != nil { return err } + if before == nil { + d.Logf("[desk-channel] an edit of %s, which nothing here records as shown: shown as a new message; "+ + "a notification it was to replace, if any, stays", m.Handle) + } case !m.Secret: var err error - if before, err = d.Store.Get(m.Handle); err != nil { + if before, err = d.recall(m.Handle); err != nil { d.Logf("[desk-channel] what is shown under this handle could not be read (%v): shown as a new message", err) before = nil } @@ -218,10 +238,84 @@ func (d *Desk) Take(subject string, body json.RawMessage) error { // Shown once and kept nowhere of this module's (a link's code): no edit or re-showing reaches it. return nil } - if err := d.Store.Put(m.Handle, s); err != nil { - d.Logf("[desk-channel] shown, and what was shown could not be kept: an edit of it will not reach it: %v", err) + return d.keep(m.Handle, s) +} + +// keep keeps what a handle shows in the store. When the store does not answer it is held in memory, where an +// edit, a reminder and a click still find it, and the error offers the work again until the store keeps it. +func (d *Desk) keep(handle string, s Shown) error { + err := d.Store.Put(handle, s) + d.mu.Lock() + defer d.mu.Unlock() + if err == nil { + delete(d.unkept, handle) + return nil } - return nil + d.unkept[handle] = s + d.Logf("[desk-channel] shown, and where it is shown could not be kept yet (%v): held in memory, so its edit "+ + "and clearing still reach it, and kept when the store answers", err) + return fmt.Errorf("where %s is shown is not kept yet: %w", handle, err) +} + +// keepUnkept keeps in the store what it did not answer for before. +func (d *Desk) keepUnkept() { + d.mu.Lock() + held := make(map[string]Shown, len(d.unkept)) + for h, s := range d.unkept { + held[h] = s + } + d.mu.Unlock() + for h, s := range held { + if err := d.Store.Put(h, s); err == nil { + d.mu.Lock() + if cur, ok := d.unkept[h]; ok && cur.Sent.Equal(s.Sent) { + delete(d.unkept, h) + } + d.mu.Unlock() + d.Logf("[desk-channel] where %s is shown is kept now", h) + } + } +} + +func (d *Desk) unkeptOf(handle string) (Shown, bool) { + d.mu.Lock() + defer d.mu.Unlock() + s, ok := d.unkept[handle] + return s, ok +} + +// recall is what a handle shows: from memory when the store did not keep it yet, else from the store. +func (d *Desk) recall(handle string) (*Shown, error) { + if s, ok := d.unkeptOf(handle); ok { + return &s, nil + } + return d.Store.Get(handle) +} + +// handles is every handle shown: the store's and those held in memory. +func (d *Desk) handles() ([]string, error) { + hs, err := d.Store.Handles() + d.mu.Lock() + defer d.mu.Unlock() + seen := map[string]bool{} + for _, h := range hs { + seen[h] = true + } + for h := range d.unkept { + if !seen[h] { + hs = append(hs, h) + } + } + if err != nil && len(d.unkept) == 0 { + return nil, err + } + return hs, nil +} + +func sameMessage(a, b asks.Message) bool { + x, err1 := json.Marshal(a) + y, err2 := json.Marshal(b) + return err1 == nil && err2 == nil && string(x) == string(y) } // show puts one message on one machine's notifier, in place of `replace` when it names one. @@ -308,12 +402,12 @@ func (d *Desk) Chosen(event, machine, ticket string) { } func (d *Desk) find(ticket string) (string, *Shown) { - handles, err := d.Store.Handles() + handles, err := d.handles() if err != nil { return "", nil } for _, h := range handles { - s, err := d.Store.Get(h) + s, err := d.recall(h) if err != nil || s == nil { continue } @@ -356,9 +450,9 @@ func (d *Desk) Started(event, machine string) int { d.lost[machine] = d.Now() d.mu.Unlock() n := 0 - if handles, err := d.Store.Handles(); err == nil { + if handles, err := d.handles(); err == nil { for _, h := range handles { - if s, err := d.Store.Get(h); err == nil && s != nil && len(s.Message.Buttons) > 0 { + if s, err := d.recall(h); err == nil && s != nil && len(s.Message.Buttons) > 0 { if _, here := s.On[machine]; here { n++ } @@ -472,6 +566,7 @@ func (d *Desk) Status() map[string]any { "it proves nothing about who clicked: any process of the operator's account can choose an action, so it answers acknowledgements only", "an answer is taken only on a machine its message was shown on; a message with buttons stays until dismissed", "what it remembers is never shown again: when a notifier started again, the answers its notifications waited for are lost; the desk says when in its standing (answers-lost-at), and the router says again what is still open and unanswered, replacing those notifications in place — nothing resolved, silenced or cancelled comes back (issue 384)", + "where a message is shown is never lost to a store that does not answer: it is held in memory, its edit and clearing still replace it in place, and its work is offered again until the store keeps it", "it can send while a named machine's notifier said lately that it is not paused; a link's code is shown only on such a machine, kept nowhere, and sent secret so the notifier keeps it out of its history", }} if s.Why != "" { diff --git a/modules/desk-channel/cmd/desk-channel/desk_test.go b/modules/desk-channel/cmd/desk-channel/desk_test.go index b4ad168f..92b37190 100644 --- a/modules/desk-channel/cmd/desk-channel/desk_test.go +++ b/modules/desk-channel/cmd/desk-channel/desk_test.go @@ -363,9 +363,57 @@ func TestAMessageSaidAgainUnderItsHandleReplacesItsNotification(t *testing.T) { } } +// downStore is a store that does not answer while down: the bus reloading its accounts (2026-10-26 09:18). +type downStore struct { + memShown + down *bool +} + +func (s downStore) Get(h string) (*Shown, error) { + if *s.down { + return nil, errors.New("nats: no responders available for request") + } + return s.memShown.Get(h) +} +func (s downStore) Put(h string, v Shown) error { + if *s.down { + return errors.New("nats: no responders available for request") + } + return s.memShown.Put(h, v) +} + // The review of PR 176: a replace that fails on a machine keeps that machine's notification on record, so a -// later edit or clearing still reaches it; failing everywhere, it is said to the router as failed. +// later edit or clearing still reaches it; failing everywhere, it is said to the router as failed. And +// (2026-10-26 09:18, an urgent "A new build of mesh-controller cannot be delivered" left on screen for hours +// after its clearing): a message shown while the store does not answer keeps where it is shown in memory, its +// work is offered again without showing it twice, its clearing replaces it in place, and it is kept once the +// store answers. func TestAFailedReplaceKeepsTheNotificationItWasToReplace(t *testing.T) { + t.Run("the store does not answer", func(t *testing.T) { + d, shown, _, store := newDesk(t, "laptop") + down := true + d.Store = downStore{store, &down} + raised, _ := json.Marshal(asks.Message{Handle: "m9", Title: "A new build cannot be delivered", Urgent: true}) + subject := asks.ChannelSubject(asks.Send, Kind) + if err := d.Take(subject, raised); err == nil { + t.Fatal("shown and not kept, and the work was not offered again") + } + if err := d.Take(subject, raised); err == nil || len(*shown) != 1 { + t.Fatalf("offered again: shown %d times, err %v", len(*shown), err) + } + cleared, _ := json.Marshal(asks.Message{Handle: "m9", Title: "Cleared: a new build can be delivered"}) + _ = d.Take(asks.ChannelSubject(asks.Edit, Kind), cleared) // still not kept: offered again + _ = d.Take(asks.ChannelSubject(asks.Edit, Kind), cleared) + if len(*shown) != 2 || (*shown)[1].args["replace_id"] != 1 { + t.Fatalf("the clearing did not replace the urgent notification: %+v", *shown) + } + down = false + take(t, d, asks.Send, asks.Message{Handle: "m10", Title: "another"}) + if s, ok := store["m9"]; !ok || s.On["laptop"] == 0 || s.Message.Title != "Cleared: a new build can be delivered" { + t.Fatalf("not kept once the store answered: %+v", store) + } + }) + d, shown, said, store := newDesk(t, "laptop", "desktop") take(t, d, asks.Send, asks.Message{Handle: "m1", Title: "2 failed services on shanks"}) first := store["m1"].On From a30c1685124656699dced991b3099589011f18db Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 11:46:09 +0100 Subject: [PATCH 2/2] desk-channel: take the work when the store does not answer, and keep what it holds at the next work and the minute's standing Review round 1: offering the work again let a redelivered raise follow its own clearing and replace it, and a redelivery is bounded where an outage is not. The work is now taken; what was shown stays in memory, recalled first, and is kept at the start of the next work and at the minute's standing, stopping at the first write the store does not answer. A desk that restarts before then loses where those are shown, and its log said how many it held. --- modules/desk-channel/cmd/desk-channel/desk.go | 57 +++++++++--------- .../cmd/desk-channel/desk_test.go | 60 +++++++++++-------- modules/desk-channel/cmd/desk-channel/main.go | 1 + 3 files changed, 65 insertions(+), 53 deletions(-) diff --git a/modules/desk-channel/cmd/desk-channel/desk.go b/modules/desk-channel/cmd/desk-channel/desk.go index 6d105eeb..e1fc3eb1 100644 --- a/modules/desk-channel/cmd/desk-channel/desk.go +++ b/modules/desk-channel/cmd/desk-channel/desk.go @@ -15,11 +15,13 @@ package main // the desk says in its standing when that happened (`answers-lost-at`), and the router, which alone knows // what is still open and unanswered, says that again under its handle — replacing the notification in place, // so its buttons work again. Nothing resolved, silenced or cancelled comes back. -// - **Where a message is shown is never lost to a store that does not answer.** A message is shown first and -// its notification ids kept after; when the state store does not answer then (the bus reloading its -// accounts, 2026-10-26 09:18: the very outage that raised the condition), the ids stay in memory, are -// recalled from there, and the work is offered again until the store keeps them. Otherwise the clearing -// of that message finds no notification to replace and the urgent one stays on screen for good. +// - **A store that does not answer does not lose where a message is shown, while the desk runs.** A message +// is shown first and its notification ids kept after; when the state store does not answer then (the bus +// reloading its accounts, 2026-10-26 09:18: the very outage that raised the condition), the work is taken +// all the same and the ids are held in memory, recalled before the store, and kept at the start of the next +// work and at the minute's standing. Otherwise the clearing of that message finds no notification to +// replace and the urgent one stays on screen for good. A desk that restarts while it still holds some loses +// where those are shown; the log says so when they are held. // - **Standing**: it can send when machines are named in its settings; which of them answered last is in its // status. A message no machine took is said to the router as failed, and tried again by it. @@ -149,11 +151,7 @@ func (d *Desk) Take(subject string, body json.RawMessage) error { d.Logf("[desk-channel] work on %s could not be read; taken and dropped", subject) return nil } - if s, ok := d.unkeptOf(m.Handle); ok && sameMessage(s.Message, m) { - // This work offered again because what it showed was not kept: kept now, not shown twice. - return d.keep(m.Handle, s) - } - d.keepUnkept() + d.KeepUnkept() machines := d.Machines() if m.Secret { // Shown only where the notifier said lately it is not paused (the review of 2026-10-09): one held back @@ -238,27 +236,31 @@ func (d *Desk) Take(subject string, body json.RawMessage) error { // Shown once and kept nowhere of this module's (a link's code): no edit or re-showing reaches it. return nil } - return d.keep(m.Handle, s) + d.keep(m.Handle, s) + return nil } // keep keeps what a handle shows in the store. When the store does not answer it is held in memory, where an -// edit, a reminder and a click still find it, and the error offers the work again until the store keeps it. -func (d *Desk) keep(handle string, s Shown) error { +// edit, a reminder and a click still find it, and kept later (KeepUnkept). The work is taken either way: offered +// again it would race the message's own later edits, and a redelivery is bounded where an outage is not. +func (d *Desk) keep(handle string, s Shown) { err := d.Store.Put(handle, s) d.mu.Lock() defer d.mu.Unlock() if err == nil { delete(d.unkept, handle) - return nil + return } d.unkept[handle] = s d.Logf("[desk-channel] shown, and where it is shown could not be kept yet (%v): held in memory, so its edit "+ - "and clearing still reach it, and kept when the store answers", err) - return fmt.Errorf("where %s is shown is not kept yet: %w", handle, err) + "and clearing still reach it, and kept when the store answers; %d held, lost if the desk restarts first", + err, len(d.unkept)) } -// keepUnkept keeps in the store what it did not answer for before. -func (d *Desk) keepUnkept() { +// KeepUnkept keeps in the store what it did not answer for before: at the start of each piece of work and at +// the minute's standing. It stops at the first write the store does not answer, so one call waits on it once. +func (d *Desk) KeepUnkept() { + d.init() d.mu.Lock() held := make(map[string]Shown, len(d.unkept)) for h, s := range d.unkept { @@ -266,14 +268,15 @@ func (d *Desk) keepUnkept() { } d.mu.Unlock() for h, s := range held { - if err := d.Store.Put(h, s); err == nil { - d.mu.Lock() - if cur, ok := d.unkept[h]; ok && cur.Sent.Equal(s.Sent) { - delete(d.unkept, h) - } - d.mu.Unlock() - d.Logf("[desk-channel] where %s is shown is kept now", h) + if err := d.Store.Put(h, s); err != nil { + return } + d.mu.Lock() + if cur, ok := d.unkept[h]; ok && sameShown(cur, s) { + delete(d.unkept, h) + } + d.mu.Unlock() + d.Logf("[desk-channel] where %s is shown is kept now", h) } } @@ -312,7 +315,7 @@ func (d *Desk) handles() ([]string, error) { return hs, nil } -func sameMessage(a, b asks.Message) bool { +func sameShown(a, b Shown) bool { x, err1 := json.Marshal(a) y, err2 := json.Marshal(b) return err1 == nil && err2 == nil && string(x) == string(y) @@ -566,7 +569,7 @@ func (d *Desk) Status() map[string]any { "it proves nothing about who clicked: any process of the operator's account can choose an action, so it answers acknowledgements only", "an answer is taken only on a machine its message was shown on; a message with buttons stays until dismissed", "what it remembers is never shown again: when a notifier started again, the answers its notifications waited for are lost; the desk says when in its standing (answers-lost-at), and the router says again what is still open and unanswered, replacing those notifications in place — nothing resolved, silenced or cancelled comes back (issue 384)", - "where a message is shown is never lost to a store that does not answer: it is held in memory, its edit and clearing still replace it in place, and its work is offered again until the store keeps it", + "a store that does not answer does not lose where a message is shown while the desk runs: it is held in memory, its edit and clearing still replace it in place, and it is kept when the store answers again; a desk restarted before then loses where those are shown, and its log said how many it held", "it can send while a named machine's notifier said lately that it is not paused; a link's code is shown only on such a machine, kept nowhere, and sent secret so the notifier keeps it out of its history", }} if s.Why != "" { diff --git a/modules/desk-channel/cmd/desk-channel/desk_test.go b/modules/desk-channel/cmd/desk-channel/desk_test.go index 92b37190..336f4d03 100644 --- a/modules/desk-channel/cmd/desk-channel/desk_test.go +++ b/modules/desk-channel/cmd/desk-channel/desk_test.go @@ -3,6 +3,7 @@ package main import ( "encoding/json" "errors" + "fmt" "os" "path/filepath" "strings" @@ -386,33 +387,40 @@ func (s downStore) Put(h string, v Shown) error { // later edit or clearing still reaches it; failing everywhere, it is said to the router as failed. And // (2026-10-26 09:18, an urgent "A new build of mesh-controller cannot be delivered" left on screen for hours // after its clearing): a message shown while the store does not answer keeps where it is shown in memory, its -// work is offered again without showing it twice, its clearing replaces it in place, and it is kept once the -// store answers. +// work is taken, its clearing replaces it in place, and it is kept once the store answers. func TestAFailedReplaceKeepsTheNotificationItWasToReplace(t *testing.T) { - t.Run("the store does not answer", func(t *testing.T) { - d, shown, _, store := newDesk(t, "laptop") - down := true - d.Store = downStore{store, &down} - raised, _ := json.Marshal(asks.Message{Handle: "m9", Title: "A new build cannot be delivered", Urgent: true}) - subject := asks.ChannelSubject(asks.Send, Kind) - if err := d.Take(subject, raised); err == nil { - t.Fatal("shown and not kept, and the work was not offered again") - } - if err := d.Take(subject, raised); err == nil || len(*shown) != 1 { - t.Fatalf("offered again: shown %d times, err %v", len(*shown), err) - } - cleared, _ := json.Marshal(asks.Message{Handle: "m9", Title: "Cleared: a new build can be delivered"}) - _ = d.Take(asks.ChannelSubject(asks.Edit, Kind), cleared) // still not kept: offered again - _ = d.Take(asks.ChannelSubject(asks.Edit, Kind), cleared) - if len(*shown) != 2 || (*shown)[1].args["replace_id"] != 1 { - t.Fatalf("the clearing did not replace the urgent notification: %+v", *shown) - } - down = false - take(t, d, asks.Send, asks.Message{Handle: "m10", Title: "another"}) - if s, ok := store["m9"]; !ok || s.On["laptop"] == 0 || s.Message.Title != "Cleared: a new build can be delivered" { - t.Fatalf("not kept once the store answered: %+v", store) - } - }) + // The raise, then its clearing, taken while the store does not answer; the store then stays down or comes + // back. Each is taken (never offered again, where a redelivered raise would follow its clearing), the + // clearing replaces the urgent notification in place, and the screen and the store end on the clearing. + for _, back := range []bool{false, true} { + t.Run(fmt.Sprintf("the store does not answer, back %v", back), func(t *testing.T) { + d, shown, _, store := newDesk(t, "laptop") + down := true + d.Store = downStore{store, &down} + raised, _ := json.Marshal(asks.Message{Handle: "m9", Title: "A new build cannot be delivered", Urgent: true}) + if err := d.Take(asks.ChannelSubject(asks.Send, Kind), raised); err != nil { + t.Fatalf("the raise was not taken: %v", err) + } + cleared, _ := json.Marshal(asks.Message{Handle: "m9", Title: "Cleared: a new build can be delivered"}) + if err := d.Take(asks.ChannelSubject(asks.Edit, Kind), cleared); err != nil { + t.Fatalf("the clearing was not taken: %v", err) + } + d.KeepUnkept() // the minute's standing, store still down + down = !back + d.KeepUnkept() + take(t, d, asks.Send, asks.Message{Handle: "m10", Title: "another"}) + if len(*shown) != 3 || (*shown)[1].args["replace_id"] != 1 || (*shown)[1].args["summary"] != "Cleared: a new build can be delivered" { + t.Fatalf("the clearing did not replace the urgent notification: %+v", *shown) + } + s, kept := store["m9"] + if back != kept || (kept && s.Message.Title != "Cleared: a new build can be delivered") { + t.Fatalf("store back %v, kept %v: %+v", back, kept, store) + } + if got, _ := d.recall("m9"); got == nil || got.Message.Title != "Cleared: a new build can be delivered" { + t.Fatalf("recalled %+v", got) + } + }) + } d, shown, said, store := newDesk(t, "laptop", "desktop") take(t, d, asks.Send, asks.Message{Handle: "m1", Title: "2 failed services on shanks"}) diff --git a/modules/desk-channel/cmd/desk-channel/main.go b/modules/desk-channel/cmd/desk-channel/main.go index 1fa72fcf..2b6b6248 100644 --- a/modules/desk-channel/cmd/desk-channel/main.go +++ b/modules/desk-channel/cmd/desk-channel/main.go @@ -100,6 +100,7 @@ func run(d *Desk) { }() go func() { for { + d.KeepUnkept() d.SayStanding() time.Sleep(time.Minute) }