From b7b34ddad756514c2abf162be18ea20da2c67b83 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 11 Oct 2026 00:30:32 +0200 Subject: [PATCH 1/3] messenger: read a condition's message before it is sent (issue 422) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The words a condition reaches the operator with are composed in the router, and nothing kept them: a send's history row holds a key and an outcome, never text, and a notification still on the desk is not in the notifier's history. So a wording could only be proven by waiting for a send and reading the operator's screen, which is why proving issue 400's tracking line stalled. messenger_preview composes what a send would carry for a condition the router holds open — the same words, the same tracking line, the same verdict — and says whether the content rule would let them out. It sends, queues and records nothing. --- modules/messenger/README.md | 1 + modules/messenger/cmd/messenger/main.go | 9 + modules/messenger/cmd/messenger/preview.go | 113 +++++++++++++ .../messenger/cmd/messenger/preview_test.go | 157 ++++++++++++++++++ modules/messenger/module.json | 1 + 5 files changed, 281 insertions(+) create mode 100644 modules/messenger/cmd/messenger/preview.go create mode 100644 modules/messenger/cmd/messenger/preview_test.go diff --git a/modules/messenger/README.md b/modules/messenger/README.md index cc76a080..722c9cbe 100644 --- a/modules/messenger/README.md +++ b/modules/messenger/README.md @@ -183,6 +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_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/main.go b/modules/messenger/cmd/messenger/main.go index c20b5041..a4711ccb 100644 --- a/modules/messenger/cmd/messenger/main.go +++ b/modules/messenger/cmd/messenger/main.go @@ -603,6 +603,15 @@ func tools(h *Holder, l *listening) []stdio.Tool { "text": str("the words (default: a line saying it is a test)"), }, 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.", + 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"), + }, + Run: func(a map[string]any) (any, error) { return h.Preview(strArg(a, "key"), strArg(a, "what")) }}, {Name: "messenger_check", Description: "Would these words be allowed to leave the mesh? Runs the content rule — no address, path or " + "secret — and answers allowed, or the class of what it carried. Sends nothing.", diff --git a/modules/messenger/cmd/messenger/preview.go b/modules/messenger/cmd/messenger/preview.go new file mode 100644 index 00000000..cf483bda --- /dev/null +++ b/modules/messenger/cmd/messenger/preview.go @@ -0,0 +1,113 @@ +package main + +// What a condition's message would say, read before it is sent (novox/hq issue 422). The words a +// condition reaches the operator with are composed here, in this router, from the condition's own plain +// words and the line on the issues that track it (tracking.go). Until this tool, the only way to read +// them was to wait for a send and look at the operator's screen: the channels keep the text of nothing +// 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 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. + +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). +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 +// nowhere. `what` is which of the messages about it (previewable, default `raised`). +func (h *Holder) Preview(key, what string) (map[string]any, error) { + key = strings.TrimSpace(key) + if key == "" { + return nil, errorf("key is required: the condition's key, as the controller names it — `mesh-controller.conditions` says which are open") + } + what = strings.TrimSpace(what) + if what == "" { + what = "raised" + } + if !previewableWhat(what) { + 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. + h.warmTracking() + + h.mu.Lock() + held, ok := h.open[key] + var r Record + if ok { + r = *held // composed from a copy: a preview changes nothing this router holds + } + open := make([]string, 0, len(h.open)) + for k := range h.open { + open = append(open, k) + } + h.mu.Unlock() + if !ok { + sort.Strings(open) + if len(open) == 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) + } + return nil, errorf("no condition %s is open here; it holds %s", key, strings.Join(open, ", ")) + } + + now := h.Now() + m := h.composeTracked(&r, what, now, 0) + out := map[string]any{ + "key": key, + "what": what, + "title": m.Title, + "body": m.Body, + "sent": "nothing: this composes the words and sends nowhere", + } + if m.Urgent { + out["urgent"] = true + } + if m.Quiet { + out["quiet"] = true + } + if line, said := h.trackingLine(&r, what, now); said { + 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 + out["carried"] = refusal.What + out["class"] = refusal.Class + } + 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 { + return true + } + } + return false +} diff --git a/modules/messenger/cmd/messenger/preview_test.go b/modules/messenger/cmd/messenger/preview_test.go new file mode 100644 index 00000000..fc9ec02b --- /dev/null +++ b/modules/messenger/cmd/messenger/preview_test.go @@ -0,0 +1,157 @@ +package main + +import ( + "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. + +// 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) { + t.Helper() + h, tg, dt, c, _ := 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.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 +} + +func TestAConditionsMessageIsReadWithoutBeingSent(t *testing.T) { + h, tg, dt, _, _ := previewHolder(t, []TrackedIssue{{Number: 352, Status: "located", Needs: "decision"}}) + + out, err := h.Preview("mesh.hand-acts.rolled-back.healer-wanted", "") + 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" { + t.Errorf("title %q", out["title"]) + } + body, _ := out["body"].(string) + if !strings.Contains(body, "Tracked as issue 352: nobody is working on it; it waits for a decision first.") { + t.Errorf("the body says nothing of the issue that tracks it: %q", body) + } + if out["tracking"] != "Tracked as issue 352: nobody is working on it; it waits for a decision first." { + t.Errorf("tracking %q", out["tracking"]) + } + if out["allowed"] != true { + t.Errorf("allowed %v, carried %v: these words may leave the mesh", out["allowed"], out["carried"]) + } + // The verdict is left out while nobody works on the issue (ADR 0285 §4), as in a send. + 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}) +} + +func TestAPreviewSaysTheSameWordsTheSendWouldCarry(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) + + out, err := h.Preview(k.Key, "reminder") + if err != nil { + t.Fatal(err) + } + // The same record, the same moment, through the sending path. + h.mu.Lock() + r := *h.open[k.Key] + 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) + } + 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) + } + 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)) + } +} + +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) + + _, err := h.Preview("machine.ace.silent", "") + if err == nil { + t.Fatal("a condition this router does not hold was previewed") + } + if !strings.Contains(err.Error(), "machine.ace.silent") || !strings.Contains(err.Error(), k.Key) { + t.Errorf("the refusal says neither what was asked nor what is held: %v", err) + } + + if _, err := h.Preview("", ""); err == nil || !strings.Contains(err.Error(), "key is required") { + t.Errorf("no key: %v", err) + } + if _, err := h.Preview(k.Key, "shouted"); err == nil || !strings.Contains(err.Error(), "shouted") { + t.Errorf("an unknown message: %v", err) + } +} + +func TestAPreviewAsksTheTrackerWhenItsWordWentCold(t *testing.T) { + asked := 0 + 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 + } + + c.pass(TrackingKept + time.Minute) // older than an answer is used for + out, err := h.Preview(k.Key, "quiet-reminder") + if err != nil { + t.Fatal(err) + } + if asked != 1 { + t.Fatalf("the tracker was asked %d times; a preview on a cold word asks once", asked) + } + if out["tracking"] != "Tracked as issue 352: nobody is working on it." { + t.Errorf("tracking %q; a preview that asked should say what it heard", out["tracking"]) + } +} + +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." + k.Raised = c.now() + h.Condition(EventRaised, k) + + out, err := h.Preview(k.Key, "") + if err != nil { + t.Fatal(err) + } + if out["allowed"] != false || out["carried"] == nil { + t.Fatalf("allowed %v, carried %v; the content rule refuses a path", out["allowed"], out["carried"]) + } +} diff --git a/modules/messenger/module.json b/modules/messenger/module.json index 1c426333..b1fde436 100644 --- a/modules/messenger/module.json +++ b/modules/messenger/module.json @@ -103,6 +103,7 @@ "messenger_status", "messenger_recent", "messenger_test", + "messenger_preview", "messenger_check", "messenger_identities", "messenger_unlink" From d00cc8baf41936c39edaf9b915c13c07abba78d5 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 11 Oct 2026 00:43:40 +0200 Subject: [PATCH 2/3] 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) } } From 15546f9d83dfd4cbbb3956745b0f2d20d150e4c6 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 11 Oct 2026 00:49:38 +0200 Subject: [PATCH 3/3] messenger: count the store's writes, and name an ask's record when it is the only one held (review of issue 422) The review's two nits. A write of bytes identical to what is stored was invisible to a comparison of content, so the test store counts its writes and the preview's tests read that count: a persist from inside Preview now fails five of them instead of one. And a router holding an ask's record and no condition said it held none, rather than saying what that key is; the branches are the other way round, with a test that holds nothing else. --- .../messenger/cmd/messenger/holder_test.go | 3 +++ modules/messenger/cmd/messenger/preview.go | 7 ++++-- .../messenger/cmd/messenger/preview_test.go | 23 +++++++++++++++---- 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/modules/messenger/cmd/messenger/holder_test.go b/modules/messenger/cmd/messenger/holder_test.go index 68092431..f4be6e3d 100644 --- a/modules/messenger/cmd/messenger/holder_test.go +++ b/modules/messenger/cmd/messenger/holder_test.go @@ -66,6 +66,8 @@ func (f *fakeChannel) Edit(id string, m Message) error { type memStore struct { recs map[string]Record recent []Sent + // puts counts the writes, so a write of bytes identical to what is stored is still seen (issue 422's review). + puts int } func (m *memStore) Put(r Record) error { @@ -73,6 +75,7 @@ func (m *memStore) Put(r Record) error { m.recs = map[string]Record{} } m.recs[r.Key] = r + m.puts++ return nil } func (m *memStore) Delete(k string) error { delete(m.recs, k); return nil } diff --git a/modules/messenger/cmd/messenger/preview.go b/modules/messenger/cmd/messenger/preview.go index 4ab44aa3..caa40e94 100644 --- a/modules/messenger/cmd/messenger/preview.go +++ b/modules/messenger/cmd/messenger/preview.go @@ -68,11 +68,14 @@ func (h *Holder) Preview(key, what string) (map[string]any, error) { 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 && len(conditions) == 0: + 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 no condition", + key, r.Origin) 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, ", ")) + 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) default: return nil, errorf("no condition %s is open here; it holds %s", key, strings.Join(conditions, ", ")) } diff --git a/modules/messenger/cmd/messenger/preview_test.go b/modules/messenger/cmd/messenger/preview_test.go index 281b6e7a..ec23f9f7 100644 --- a/modules/messenger/cmd/messenger/preview_test.go +++ b/modules/messenger/cmd/messenger/preview_test.go @@ -33,9 +33,9 @@ func previewHolder(t *testing.T, issues []TrackedIssue) (*Holder, *fakeChannel, // 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 + sends, queued, recent, refusals, puts int + stored Record + held Record } func mark(t *testing.T, h *Holder, tg, dt *fakeChannel, st *memStore, key string) unmoved { @@ -47,7 +47,7 @@ func mark(t *testing.T, h *Holder, tg, dt *fakeChannel, st *memStore, key string 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]} + refusals: len(h.refusals), puts: st.puts, 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) { @@ -66,6 +66,9 @@ func (was unmoved) check(t *testing.T, h *Holder, tg, dt *fakeChannel, st *memSt if now.refusals != was.refusals { t.Errorf("a preview recorded %d refusal(s): only a send does that", now.refusals-was.refusals) } + if now.puts != was.puts { + t.Errorf("a preview wrote to the store %d time(s), even if the bytes were the same", now.puts-was.puts) + } if !reflect.DeepEqual(now.stored, was.stored) { t.Errorf("a preview wrote the record: %+v", now.stored) } @@ -204,9 +207,19 @@ func TestAPreviewOfWhatThisRouterDoesNotHoldIsRefusedByName(t *testing.T) { 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") { + if err == nil || !strings.Contains(err.Error(), "not a condition") || !strings.Contains(err.Error(), k.Key) { t.Errorf("an ask's record previewed as a condition: %v", err) } + + // A router holding an ask and no condition says what that key is, not that it holds nothing. + bare, _, _, _, _ := newHolder(t) + bare.mu.Lock() + bare.open["ask.someone.2"] = &Record{Key: "ask.someone.2", Origin: "ask", Severity: Urgent, Summary: "a question"} + bare.mu.Unlock() + _, err = bare.Preview("ask.someone.2", "") + if err == nil || !strings.Contains(err.Error(), "not a condition") { + t.Errorf("the only record held is an ask's: %v", err) + } } func TestAPreviewAsksTheTrackerWhenItsWordWentCold(t *testing.T) {