From d00cc8baf41936c39edaf9b915c13c07abba78d5 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 11 Oct 2026 00:43:40 +0200 Subject: [PATCH] messenger: the preview is the sending path's own walk (review of issue 422) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The review of #192 found three faults, all verified by running. The tests did not prove the claim the change leads with: a probe that sent, queued, recorded and wrote from inside Preview still passed, because the condition they used was a warning needing nothing, which is held rather than sent. They now hold an urgent condition, snapshot what a send would move — the channels, the queue, the history, the refusals, the stored record and the record held — assert none of it moved, and tick once more. Every one of those probes now fails the tests. `cleared` and `silenced` said wording no send would carry: an open record has no clearing time, so a condition open four hours cleared "after a moment", and no silence, so a silence read "until Mon 1 Jan 00:17". A clearing is now composed as if it cleared now, which the answer says; a silence is refused unless the condition is under one. The preview composed only its own words, while a send reduces a message whose words the content rule refuses. The escalation walk is now one function, allowedWords, which `words` calls for a send and the preview calls alone — so the words read are the words that would leave, and reading them records no refusal and states nothing on the bus. withTracking hands back the line it put in, so the preview reports that line instead of deciding the rule a second time; and a record that is not a condition, or one carrying an open ask, is refused or says that a channel's message carries more. --- modules/messenger/README.md | 2 +- modules/messenger/cmd/messenger/holder.go | 64 +++-- modules/messenger/cmd/messenger/main.go | 6 +- modules/messenger/cmd/messenger/preview.go | 103 +++++---- .../messenger/cmd/messenger/preview_test.go | 218 ++++++++++++++---- modules/messenger/cmd/messenger/tracking.go | 8 +- .../messenger/cmd/messenger/tracking_test.go | 15 +- 7 files changed, 291 insertions(+), 125 deletions(-) diff --git a/modules/messenger/README.md b/modules/messenger/README.md index 722c9cbe..bf6463df 100644 --- a/modules/messenger/README.md +++ b/modules/messenger/README.md @@ -183,7 +183,7 @@ ends takes them away everywhere; its expiry is said with its day when it is not | `messenger_status` | whether the operator can be reached and why not, per channel kind | | `messenger_recent` | the recent sends, edits, digests, answers and failures | | `messenger_test` | a test message now, to one kind or every one | -| `messenger_preview` | what a condition's message would say, tracking line and all, without sending it | +| `messenger_preview` | what a condition's message would say, tracking line and all, without sending it: the words a send would carry, reduced where the content rule refuses its own | | `messenger_check` | whether some words may leave the mesh | | `messenger_identities` | the accounts linked as the operator, and from when each approves | | `messenger_unlink` | unlink a kind's account, said on every channel | diff --git a/modules/messenger/cmd/messenger/holder.go b/modules/messenger/cmd/messenger/holder.go index 298698b2..755c224e 100644 --- a/modules/messenger/cmd/messenger/holder.go +++ b/modules/messenger/cmd/messenger/holder.go @@ -874,19 +874,9 @@ func (h *Holder) askLines(rec *AskRecord) []string { // until what remains may leave. The refusal is recorded and stated once per record. func (h *Holder) words(r *Record, what string) Message { now := h.Now() - if r.Refused != "" { - // Refused before: its words stay withheld in every later message too. - for w := 1; w <= 3; w++ { - m := h.composeTracked(r, what, now, w) - if _, ok := Check(m.Text()); ok { - return m - } - } - } - m := h.composeTracked(r, what, now, 0) - refusal, ok := Check(m.Text()) - if ok { - return m + said, _, refusal, ok := h.allowedWords(r, what, now) + if ok || r.Refused != "" { + return said } h.mu.Lock() r.Refused = refusal.What @@ -907,26 +897,56 @@ func (h *Holder) words(r *Record, what string) Message { h.Logf("[messenger] could not state the refusal on the bus: %v", err) } } - for w := 1; w <= 3; w++ { - m := h.composeTracked(r, what, now, w) - if _, ok := Check(m.Text()); ok { - return m - } - } - return withheld(refusal.What, r.Severity == Urgent) + return said } // composeTracked is compose with the line on the issues that track the condition (tracking.go), so the content // rule judges the message as it is sent. -func (h *Holder) composeTracked(r *Record, what string, now time.Time, withhold int) Message { +func (h *Holder) composeTracked(r *Record, what string, now time.Time, withhold int) (Message, string) { m := compose(r, what, now, h.zone(), withhold) if !h.tracksAsked() || withhold >= 2 { - return m + return m, "" } issues, known := h.trackedBy(r.Key) return withTracking(m, r, what, issues, known, now, h.zone()) } +// allowedWords is the message the content rule leaves of a record: its own words when they may leave the +// mesh, else the same message composed again with less of it until what remains may, else the withheld +// one. It answers the tracking line it carries, what its own words carried, and whether those words are +// what it says. **It changes nothing**: `words` is this walk with the refusal recorded and stated once, +// and `Preview` (preview.go) is this walk alone, so a read of a wording cannot alter what a send says. +func (h *Holder) allowedWords(r *Record, what string, now time.Time) (Message, string, Refusal, bool) { + if r.Refused != "" { + // Refused before: its words stay withheld in every later message too. + for w := 1; w <= 3; w++ { + if m, line := h.composeTracked(r, what, now, w); allowed(m) { + return m, line, Refusal{What: r.Refused}, false + } + } + } + m, line := h.composeTracked(r, what, now, 0) + refusal, ok := Check(m.Text()) + if ok { + return m, line, Refusal{}, true + } + // Composed from a copy carrying the reason, because a withheld message says what it carried. + reduced := *r + reduced.Refused = refusal.What + for w := 1; w <= 3; w++ { + if m, line := h.composeTracked(&reduced, what, now, w); allowed(m) { + return m, line, refusal, false + } + } + return withheld(refusal.What, r.Severity == Urgent), "", refusal, false +} + +// allowed says whether a message's words may leave the mesh. +func allowed(m Message) bool { + _, ok := Check(m.Text()) + return ok +} + // deliver sends a record's message where its severity routes it. func (h *Holder) channels() []Channel { diff --git a/modules/messenger/cmd/messenger/main.go b/modules/messenger/cmd/messenger/main.go index a4711ccb..e0f2499f 100644 --- a/modules/messenger/cmd/messenger/main.go +++ b/modules/messenger/cmd/messenger/main.go @@ -605,8 +605,10 @@ func tools(h *Holder, l *listening) []stdio.Tool { Run: func(a map[string]any) (any, error) { return h.Test(strArg(a, "channel"), strArg(a, "text")), nil }}, {Name: "messenger_preview", Description: "What a condition's message would say, without sending it: the title, the words, the " + - "line on the issues that track it, and whether the content rule would let them leave the mesh. " + - "Composed exactly as it would be sent, from a condition this router holds open; sends nothing.", + "line on the issues that track it, and whether the content rule would let them leave the mesh — " + + "where they may not, the reduced message a send would carry instead. Composed for a condition this " + + "router holds open, by the walk a send composes with; a clearing as if it cleared now, a silence " + + "only while there is one. It sends, queues and records nothing.", Input: map[string]any{ "key": str("the condition's key, as the controller names it, e.g. module.openrazer.laptop.relogin-needed"), "what": str("which message about it: raised (default), reminder, quiet-reminder, escalated, reopened, cleared, silenced"), diff --git a/modules/messenger/cmd/messenger/preview.go b/modules/messenger/cmd/messenger/preview.go index cf483bda..4ab44aa3 100644 --- a/modules/messenger/cmd/messenger/preview.go +++ b/modules/messenger/cmd/messenger/preview.go @@ -7,22 +7,27 @@ package main // they sent (`Sent` holds a key and an outcome, never words), and a notification still on the desk is // not in the notifier's history. So a wording could not be proven without nagging the operator. // -// It composes exactly what `composeTracked` composes for a send — the same words, the same tracking -// line, the same verdict — and sends nothing, queues nothing and records nothing. It also says whether -// the content rule would let those words leave the mesh, which is the other half of what a send decides. +// It is the sending path's own walk and nothing beside it: `allowedWords` (holder.go), which composes +// the message, carries the tracking line into it and reduces it where the content rule refuses its +// words — the same function `words` calls for a send, so a preview cannot drift from one. It records no +// refusal, states nothing on the bus, queues nothing, sends nothing and changes no record: the walk runs +// against a copy. // -// It reads the conditions this router holds open. A key it does not hold is refused by name: a -// condition that is not open has no message, and one invented here would prove nothing about the real -// one. +// What it does not say, because a channel adds it where a condition carries an open ask (`sayOn`): who +// asks, what each answer does, and the answers themselves. A preview of such a condition says that its +// message carries more, rather than letting the words read as the whole of it. +// +// Two messages are about a moment that has not come. A clearing is composed as if the condition cleared +// now, which the answer says, because a clearing composed from an open record would say "after a moment" +// for a condition open four hours. A silence is refused unless the condition is under one: a silence +// message says until when, and a condition nobody silenced has no such moment to say. import ( "sort" "strings" - "time" ) -// The messages a condition is said with, as `what` names them through the sending path (words.go). -// `raised` is the first message about it; `cleared` and `silenced` carry no tracking line (ADR 0285 §3). +// The messages a condition is said with, as `what` names them through the sending path (holder.go). var previewable = []string{"raised", "reminder", "quiet-reminder", "escalated", "reopened", "cleared", "silenced"} // Preview is the message for a condition this router holds, composed as it would be sent, and sent @@ -40,31 +45,51 @@ func (h *Holder) Preview(key, what string) (map[string]any, error) { return nil, errorf("what %q is not one of the messages a condition is said with: %s", what, strings.Join(previewable, ", ")) } - // Asked outside the work, as the router asks it on its own tick: a preview on a cold tracking word - // would say the tracking could not be checked, which is true of this moment and not of a send. + // Asked outside the work, as every tick that composes a message warms it: a preview on a cold + // tracking word would say the tracking could not be checked, which is true of this instant and not + // of the send it is read for. h.warmTracking() + now := h.Now() h.mu.Lock() - held, ok := h.open[key] + held, holding := h.open[key] var r Record - if ok { - r = *held // composed from a copy: a preview changes nothing this router holds + if holding { + r = *held // the walk runs against a copy: a preview changes no record this router holds } - open := make([]string, 0, len(h.open)) - for k := range h.open { - open = append(open, k) + var conditions []string + for k, rec := range h.open { + if rec.Origin == "condition" { + conditions = append(conditions, k) + } } h.mu.Unlock() - if !ok { - sort.Strings(open) - if len(open) == 0 { + + if !holding || r.Origin != "condition" { + sort.Strings(conditions) + switch { + case len(conditions) == 0: return nil, errorf("no condition %s is open here, and this router holds none: `mesh-controller.conditions` says what the controller holds open", key) + case holding: + return nil, errorf("%s is not a condition but this router's own %s record, which is said as part of an ask, never on its own; it holds the conditions %s", + key, r.Origin, strings.Join(conditions, ", ")) + default: + return nil, errorf("no condition %s is open here; it holds %s", key, strings.Join(conditions, ", ")) } - return nil, errorf("no condition %s is open here; it holds %s", key, strings.Join(open, ", ")) + } + if what == "silenced" && !r.silenced(now) { + return nil, errorf("%s is under no silence, and a silence message says until when it is quiet: there is no such message to read", key) + } + if what == "cleared" { + // As if it cleared now: the clearing says how long it was open, which an open record cannot say. + r.Cleared = now } - now := h.Now() - m := h.composeTracked(&r, what, now, 0) + asking := h.openAsk(&r) != nil // askOf takes the same lock, so this is asked outside it + m, line, refusal, ownWords := h.allowedWords(&r, what, now) + if r.Answered != "" { + m = h.answeredWords(&r, m) + } out := map[string]any{ "key": key, "what": what, @@ -78,31 +103,27 @@ func (h *Holder) Preview(key, what string) (map[string]any, error) { if m.Quiet { out["quiet"] = true } - if line, said := h.trackingLine(&r, what, now); said { + if line != "" { out["tracking"] = line } - if refusal, allowed := Check(m.Text()); allowed { - out["allowed"] = true - } else { - // A send would say less instead (compose with a withhold level), never these words. - out["allowed"] = false + if what == "cleared" { + out["as-if"] = "it cleared now: how long it was open is said from this moment" + } + if asking { + out["carries-more"] = "this condition has an open ask, so a channel's message also carries who asks, what each answer does, and the answers themselves" + } + out["allowed"] = ownWords + if !ownWords { + // What stands above is what would leave; its own words would not, and this says why. out["carried"] = refusal.What - out["class"] = refusal.Class + if refusal.Class != "" { + out["class"] = refusal.Class + } + out["instead"] = "its own words may not leave the mesh, so this is the message as reduced for sending" } return out, nil } -// trackingLine is the line on the issues that track the condition, and whether this message carries one: -// a clearing and a silence say none, and nor does a router that cannot ask the tracker. -func (h *Holder) trackingLine(r *Record, what string, now time.Time) (string, bool) { - if !h.tracksAsked() || r.Origin != "condition" || what == "cleared" || what == "silenced" { - return "", false - } - issues, known := h.trackedBy(r.Key) - line, _ := trackingWords(issues, known, now, h.zone()) - return line, line != "" -} - func previewableWhat(what string) bool { for _, w := range previewable { if w == what { diff --git a/modules/messenger/cmd/messenger/preview_test.go b/modules/messenger/cmd/messenger/preview_test.go index fc9ec02b..281b6e7a 100644 --- a/modules/messenger/cmd/messenger/preview_test.go +++ b/modules/messenger/cmd/messenger/preview_test.go @@ -1,43 +1,95 @@ package main import ( + "reflect" "strings" "testing" "time" ) -// A condition's message can be read before it is sent (novox/hq issue 422): the same words a send would -// carry, the tracking line among them, and nothing goes out while they are read. +// A condition's message can be read before it is sent (novox/hq issue 422): the words a send would +// carry, the tracking line among them, and nothing of this router moved while they were read. -// previewHolder is a router holding one raised condition that needs nothing, with the issues given -// tracking its key. -func previewHolder(t *testing.T, issues []TrackedIssue) (*Holder, *fakeChannel, *fakeChannel, *clock, Condition) { +// previewHolder is a router holding one urgent condition — one a send goes out for at once, so a preview +// that sends, queues or records anything is caught — with the issues given tracking its key. +func previewHolder(t *testing.T, issues []TrackedIssue) (*Holder, *fakeChannel, *fakeChannel, *clock, *memStore, Condition) { t.Helper() - h, tg, dt, c, _ := newHolder(t) + h, tg, dt, c, st := newHolder(t) h.Tracking = func() (map[string][]TrackedIssue, error) { return map[string][]TrackedIssue{"mesh.hand-acts.rolled-back.healer-wanted": issues}, nil } - k := cond("mesh.hand-acts.rolled-back.healer-wanted", Warning, "rolled-back was repaired by hand 4 times in 14 days") + k := cond("mesh.hand-acts.rolled-back.healer-wanted", Urgent, "rolled-back was repaired by hand 4 times in 14 days") k.Explanation = "Nothing for you to do. A person repaired rolled-back by hand 4 times in 14 days, so an automatic repair is wanted for it. Nothing is broken now." k.Needs = "" k.Raised = c.now() h.Condition(EventRaised, k) settle(h, c) - tg.sends, dt.sends = nil, nil - return h, tg, dt, c, k + if len(tg.sends) == 0 && len(dt.sends) == 0 { + t.Fatal("the condition this is read from was never sent, so a preview sending something would not show") + } + return h, tg, dt, c, st, k +} + +// unmoved fails when anything a send would move has moved: the sends, the queue, the history, what is +// stored, and the record itself. +type unmoved struct { + sends, queued, recent, refusals int + stored Record + held Record +} + +func mark(t *testing.T, h *Holder, tg, dt *fakeChannel, st *memStore, key string) unmoved { + t.Helper() + h.mu.Lock() + defer h.mu.Unlock() + queued := 0 // the outbox is held per channel, never per condition + for _, list := range h.outbox { + queued += len(list) + } + return unmoved{sends: len(tg.sends) + len(dt.sends), queued: queued, recent: len(h.recent), + refusals: len(h.refusals), stored: st.recs[key], held: *h.open[key]} +} + +func (was unmoved) check(t *testing.T, h *Holder, tg, dt *fakeChannel, st *memStore, c *clock, key string) { + t.Helper() + // A queued message waits for a tick, so the queue is read and then given one: either would catch it. + now := mark(t, h, tg, dt, st, key) + if now.sends != was.sends { + t.Errorf("a preview sent %d message(s)", now.sends-was.sends) + } + if now.queued != was.queued { + t.Errorf("a preview queued %d message(s)", now.queued-was.queued) + } + if now.recent != was.recent { + t.Errorf("a preview recorded %d send(s)", now.recent-was.recent) + } + if now.refusals != was.refusals { + t.Errorf("a preview recorded %d refusal(s): only a send does that", now.refusals-was.refusals) + } + if !reflect.DeepEqual(now.stored, was.stored) { + t.Errorf("a preview wrote the record: %+v", now.stored) + } + if !reflect.DeepEqual(now.held, was.held) { + t.Errorf("a preview changed the record held: %+v", now.held) + } + settle(h, c) + if after := mark(t, h, tg, dt, st, key); after.sends != was.sends { + t.Errorf("a preview queued a message, which the next tick sent: %d", after.sends-was.sends) + } } func TestAConditionsMessageIsReadWithoutBeingSent(t *testing.T) { - h, tg, dt, _, _ := previewHolder(t, []TrackedIssue{{Number: 352, Status: "located", Needs: "decision"}}) + h, tg, dt, c, st, k := previewHolder(t, []TrackedIssue{{Number: 352, Status: "located", Needs: "decision"}}) + was := mark(t, h, tg, dt, st, k.Key) - out, err := h.Preview("mesh.hand-acts.rolled-back.healer-wanted", "") + out, err := h.Preview(k.Key, "") if err != nil { t.Fatal(err) } if out["what"] != "raised" { t.Errorf("what %v; a preview with none asked is the first message", out["what"]) } - if out["title"] != "mesh hand-acts rolled-back healer-wanted" { + if out["title"] != "Urgent: mesh hand-acts rolled-back healer-wanted" { t.Errorf("title %q", out["title"]) } body, _ := out["body"].(string) @@ -54,54 +106,83 @@ func TestAConditionsMessageIsReadWithoutBeingSent(t *testing.T) { if strings.Contains(body, "Nothing for you to do.") { t.Errorf("the preview reassures where nothing is being done: %q", body) } - if len(tg.sends) != 0 || len(dt.sends) != 0 { - t.Fatalf("a preview sent something: telegram %d, desktop %d", len(tg.sends), len(dt.sends)) - } keyless(t, Message{Title: out["title"].(string), Body: body}) + was.check(t, h, tg, dt, st, c, k.Key) } -func TestAPreviewSaysTheSameWordsTheSendWouldCarry(t *testing.T) { +func TestAPreviewSaysTheWordsTheChannelWasGiven(t *testing.T) { issues := []TrackedIssue{{Number: 386, Status: "fixing", Worker: "fix-386@g14", Since: at(21, 10)}} - h, tg, dt, c, k := previewHolder(t, issues) + h, tg, dt, c, st, k := previewHolder(t, issues) + sent := dt.sends[len(dt.sends)-1] // the message the desk was actually handed + was := mark(t, h, tg, dt, st, k.Key) - out, err := h.Preview(k.Key, "reminder") + out, err := h.Preview(k.Key, "") if err != nil { t.Fatal(err) } - // The same record, the same moment, through the sending path. + if out["title"] != sent.Title || out["body"] != sent.Body { + t.Fatalf("preview %q / %q; the channel was given %q / %q", out["title"], out["body"], sent.Title, sent.Body) + } + if !strings.Contains(sent.Body, "Tracked as issue 386: fix-386 on g14 is fixing it since ") { + t.Fatalf("the message this is compared against carries no tracking line: %q", sent.Body) + } + was.check(t, h, tg, dt, st, c, k.Key) +} + +func TestAClearingIsPreviewedAsIfItClearedNow(t *testing.T) { + h, tg, dt, c, st, k := previewHolder(t, []TrackedIssue{{Number: 352, Status: "located"}}) + c.pass(4 * time.Hour) + was := mark(t, h, tg, dt, st, k.Key) + + out, err := h.Preview(k.Key, "cleared") + if err != nil { + t.Fatal(err) + } + // Composed from the open record, this said "after a moment": the clearing's own time is zero, so the + // time it was open came out negative. + if out["title"] != "Resolved: mesh hand-acts rolled-back healer-wanted, after 4.0 h" { + t.Errorf("title %q; a clearing says how long it was open", out["title"]) + } + if out["as-if"] == nil { + t.Errorf("the answer does not say the clearing is composed as if it cleared now: %v", out) + } + if _, said := out["tracking"]; said { + t.Errorf("a clearing says a tracking line: %v", out["tracking"]) + } + was.check(t, h, tg, dt, st, c, k.Key) +} + +func TestASilenceIsPreviewedOnlyWhileThereIsOne(t *testing.T) { + h, tg, dt, c, st, k := previewHolder(t, []TrackedIssue{{Number: 352, Status: "located"}}) + + // Under no silence, a silence message would say a moment that does not exist. + _, err := h.Preview(k.Key, "silenced") + if err == nil || !strings.Contains(err.Error(), "under no silence") { + t.Fatalf("a silence previewed without one: %v", err) + } + h.mu.Lock() - r := *h.open[k.Key] + h.open[k.Key].SilencedTill = c.now().Add(7 * 24 * time.Hour) + h.open[k.Key].SilencedWhy = "the operator knows" h.mu.Unlock() - want := h.composeTracked(&r, "reminder", c.now(), 0) - if out["title"] != want.Title || out["body"] != want.Body { - t.Fatalf("preview %q / %q; a send would say %q / %q", out["title"], out["body"], want.Title, want.Body) + was := mark(t, h, tg, dt, st, k.Key) + + out, err := h.Preview(k.Key, "silenced") + if err != nil { + t.Fatal(err) } - if !strings.Contains(want.Body, "Tracked as issue 386: fix-386 on g14 is fixing it since ") { - t.Fatalf("the message this is compared against carries no tracking line: %q", want.Body) + title, _ := out["title"].(string) + if !strings.HasPrefix(title, "Silenced until ") || strings.Contains(title, "Jan 1 ") || strings.Contains(title, " 1 Jan ") { + t.Errorf("title %q; a silence says until when it is quiet", title) } - if len(tg.sends) != 0 || len(dt.sends) != 0 { - t.Fatalf("a preview sent something: telegram %d, desktop %d", len(tg.sends), len(dt.sends)) + if _, said := out["tracking"]; said { + t.Errorf("a silence says a tracking line: %v", out["tracking"]) } + was.check(t, h, tg, dt, st, c, k.Key) } -func TestAClearingAndASilencePreviewWithoutATrackingLine(t *testing.T) { - h, _, _, _, k := previewHolder(t, []TrackedIssue{{Number: 352, Status: "located"}}) - for _, what := range []string{"cleared", "silenced"} { - out, err := h.Preview(k.Key, what) - if err != nil { - t.Fatal(err) - } - if _, said := out["tracking"]; said { - t.Errorf("%s says a tracking line: %v", what, out["tracking"]) - } - if body, _ := out["body"].(string); strings.Contains(body, "issue") { - t.Errorf("%s names an issue: %q", what, body) - } - } -} - -func TestAPreviewOfAConditionTheRouterDoesNotHoldIsRefusedByName(t *testing.T) { - h, _, _, _, k := previewHolder(t, nil) +func TestAPreviewOfWhatThisRouterDoesNotHoldIsRefusedByName(t *testing.T) { + h, _, _, _, _, k := previewHolder(t, nil) _, err := h.Preview("machine.ace.silent", "") if err == nil { @@ -117,11 +198,20 @@ func TestAPreviewOfAConditionTheRouterDoesNotHoldIsRefusedByName(t *testing.T) { if _, err := h.Preview(k.Key, "shouted"); err == nil || !strings.Contains(err.Error(), "shouted") { t.Errorf("an unknown message: %v", err) } + + // An ask's own record is held here too, and is said as part of an ask, never as a condition. + h.mu.Lock() + h.open["ask.someone.1"] = &Record{Key: "ask.someone.1", Origin: "ask", Severity: Urgent, Summary: "a question"} + h.mu.Unlock() + _, err = h.Preview("ask.someone.1", "") + if err == nil || !strings.Contains(err.Error(), "not a condition") { + t.Errorf("an ask's record previewed as a condition: %v", err) + } } func TestAPreviewAsksTheTrackerWhenItsWordWentCold(t *testing.T) { asked := 0 - h, _, _, c, k := previewHolder(t, []TrackedIssue{{Number: 352, Status: "located"}}) + h, _, _, c, _, k := previewHolder(t, []TrackedIssue{{Number: 352, Status: "located"}}) h.Tracking = func() (map[string][]TrackedIssue, error) { asked++ return map[string][]TrackedIssue{k.Key: {{Number: 352, Status: "located"}}}, nil @@ -140,12 +230,18 @@ func TestAPreviewAsksTheTrackerWhenItsWordWentCold(t *testing.T) { } } -func TestAPreviewSaysWhenTheWordsWouldNotBeAllowedOut(t *testing.T) { - h, _, _, c, _ := previewHolder(t, nil) - k := cond("machine.ace.silent", Warning, "the home server has not been heard for 15 min") - k.Explanation = "Needs you: look at /home/someone/secrets.txt on it." +func TestAPreviewSaysTheReducedWordsAndWhyWhenItsOwnMayNotLeave(t *testing.T) { + h, tg, dt, c, st, _ := previewHolder(t, nil) + stated := 0 + h.Emit = func(string, any) error { stated++; return nil } + // A warning that needs nothing is held, not sent, so this refusal is one no send has met yet. + k := cond("machine.ace.unreadable", Warning, "a file on it could not be read") + k.Explanation = "Nothing for you to do. A file at /home/someone/secrets.txt could not be read." + k.Needs = "" k.Raised = c.now() h.Condition(EventRaised, k) + settle(h, c) + was := mark(t, h, tg, dt, st, k.Key) out, err := h.Preview(k.Key, "") if err != nil { @@ -154,4 +250,26 @@ func TestAPreviewSaysWhenTheWordsWouldNotBeAllowedOut(t *testing.T) { if out["allowed"] != false || out["carried"] == nil { t.Fatalf("allowed %v, carried %v; the content rule refuses a path", out["allowed"], out["carried"]) } + // The words said are the ones that would go out, not the refused ones. + body, _ := out["body"].(string) + if strings.Contains(body, "/home/") { + t.Errorf("the preview says words no send would carry: %q", body) + } + if !strings.Contains(body, "withheld") { + t.Errorf("body %q; a reduced message says that part of it was withheld", body) + } + if !strings.Contains(body, "a file path") { + t.Errorf("body %q; a reduced message says what it carried", body) + } + if out["instead"] == nil { + t.Errorf("the answer does not say these are the reduced words: %v", out) + } + // Reading it recorded no refusal, stated nothing on the bus, and left the record's own words alone. + if stated != 0 { + t.Errorf("a preview stated %d fact(s) on the bus", stated) + } + if was.held.Refused != "" { + t.Fatalf("the record was already refused before the preview: %q", was.held.Refused) + } + was.check(t, h, tg, dt, st, c, k.Key) } diff --git a/modules/messenger/cmd/messenger/tracking.go b/modules/messenger/cmd/messenger/tracking.go index eeea1f24..9e0a484e 100644 --- a/modules/messenger/cmd/messenger/tracking.go +++ b/modules/messenger/cmd/messenger/tracking.go @@ -159,9 +159,11 @@ const ( // withTracking is a condition's message with the line on the issues that track it, and its verdict changed where // an issue waits for the operator or nobody works on any of them. Cleared and silenced messages, and one withheld // whole, say nothing of it. -func withTracking(m Message, r *Record, what string, issues []TrackedIssue, known bool, now time.Time, loc *time.Location) Message { +// It answers the line it put in the message, empty when the message carries none, so whoever composed it +// never has to work out the rule a second time (novox/hq issue 422). +func withTracking(m Message, r *Record, what string, issues []TrackedIssue, known bool, now time.Time, loc *time.Location) (Message, string) { if r.Origin != "condition" || what == "cleared" || what == "silenced" || m.Title == "A mesh message was withheld" { - return m + return m, "" } line, verdict := trackingWords(issues, known, now, loc) // since is the last line, which never holds a line break; the explanation is everything before it. @@ -184,7 +186,7 @@ func withTracking(m Message, r *Record, what string, issues []TrackedIssue, know } } m.Body = strings.Join(parts, "\n") - return m + return m, line } // needsAnswer is the "Needs you:" sentence for the issues that wait for the operator's answer. diff --git a/modules/messenger/cmd/messenger/tracking_test.go b/modules/messenger/cmd/messenger/tracking_test.go index 47d2364a..fbf719c3 100644 --- a/modules/messenger/cmd/messenger/tracking_test.go +++ b/modules/messenger/cmd/messenger/tracking_test.go @@ -26,7 +26,8 @@ func trackedMessage(issues []TrackedIssue, known bool) Message { Explanation: "Nothing for you to do. A person repaired rolled-back by hand 4 times in 14 days, so an automatic repair is wanted for it. Nothing is broken now.", Raised: trackNow.Add(-26 * time.Hour)} m := compose(r, "quiet-reminder", trackNow, time.UTC, 0) - return withTracking(m, r, "quiet-reminder", issues, known, trackNow, time.UTC) + out, _ := withTracking(m, r, "quiet-reminder", issues, known, trackNow, time.UTC) + return out } func TestAConditionsMessageSaysWhereTheIssueThatTracksItStands(t *testing.T) { @@ -129,7 +130,7 @@ func TestNothingForYouToDoIsNeverSaidWhileTheIssueWaitsOnTheOperatorOrNobodyWork func TestAVerdictThatNeedsTheOperatorStands(t *testing.T) { r := &Record{Key: "module.mounts.shanks.unhealthy", Origin: "condition", Severity: Urgent, Headline: "mounts not working on shanks", Explanation: "Needs you: give the share's login. mounts on shanks is not healthy.", Raised: trackNow.Add(-time.Hour)} - m := withTracking(compose(r, "raised", trackNow, time.UTC, 0), r, "raised", + m, _ := withTracking(compose(r, "raised", trackNow, time.UTC, 0), r, "raised", []TrackedIssue{{Number: 386, Status: "open", Needs: "operator"}}, true, trackNow, time.UTC) want := "Needs you: give the share's login. mounts on shanks is not healthy.\nTracked as issue 386: it waits for your answer.\nSince 18:30." if m.Body != want { @@ -144,14 +145,16 @@ func TestAClearingASilenceAndANoticeSayNoIssue(t *testing.T) { Cleared: trackNow, SilencedTill: trackNow.Add(time.Hour)} for _, what := range []string{"cleared", "silenced"} { m := compose(r, what, trackNow, time.UTC, 0) - if got := withTracking(m, r, what, issues, true, trackNow, time.UTC); got.Body != m.Body { - t.Errorf("%s: %q", what, got.Body) + got, line := withTracking(m, r, what, issues, true, trackNow, time.UTC) + if got.Body != m.Body || line != "" { + t.Errorf("%s: %q, line %q", what, got.Body, line) } } n := &Record{Key: "n", Origin: "notify", Headline: "a notice", Raised: trackNow} m := compose(n, "raised", trackNow, time.UTC, 0) - if got := withTracking(m, n, "raised", issues, true, trackNow, time.UTC); got.Body != m.Body { - t.Errorf("notice: %q", got.Body) + got, line := withTracking(m, n, "raised", issues, true, trackNow, time.UTC) + if got.Body != m.Body || line != "" { + t.Errorf("notice: %q, line %q", got.Body, line) } }