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) }