mail_send's ask: judge messenger's real words, carry the content rule at run time, match display-name recipients (issue 868, review round 1)

Review round 1: the test judged Headline+Explanation+Details, but messenger refuses an ask as a whole on
Headline, Explanation, OnExpiry and each option's Label and Does alone (asks.go's TakeAsk); Details is
filtered line by line instead (holder.go's sayOn). send.go's Ask() now builds that exact judged text and
checks it with messenger's real content rule, carried at run time as content_rule.go (messenger's
content.go, renamed, byte-identical, pinned by copies_test.go) rather than only in a test copy. When a
subject or first line the masking does not catch (an IP, a hardware address, a path-shaped reference, a
hex hash, ...) would still refuse the whole ask, the Explanation drops the subject and first lines and
points at Details instead, so the ask always reaches the operator. A recipient written with a display
name ('Name <addr>') is now matched against the operator's own accounts, and masked, by its address alone
(net/mail.ParseAddress).

modules/mesh-mail/cmd/mesh-mail/ask_test.go

@@ -3,27 +3,43 @@ package main
 // mail_send raises its ask on the operator channel, and messenger's content rule (ADR 0253 §3) refuses any
 // message that carries a mail address, a URL or a bare host name. ADR 0361 §2 needs the ask to show the
 // account, the recipients, the subject and the first lines regardless — so the ask must say all of that in
-// a form the content rule lets through (novox/hq issue 868, "Decided" note on issue 868). These tests build
-// real asks through mail_send and judge them with the real rule, copied verbatim as content_test.go.
+// a form the content rule lets through (novox/hq issue 868, "Decided" note and review round 1). These tests
+// build real asks through mail_send and judge them with the real rule, copied verbatim as content_rule.go.
 
 import (
 	"encoding/json"
 	"strings"
 	"testing"
+
+	"git.novox.be/novox/mesh-sdk/go/asks"
 )
 
-// wholeAsk is every word the operator sees for one ask, the same text messenger's content rule judges as a
-// whole (the asks router hands it Headline, Explanation and Details together).
+// wholeAsk is every word the operator can ever see for one ask: its Headline, Explanation and Details
+// together. Useful for a readability check (nothing unmasked is visible anywhere), but never for judging
+// whether messenger would refuse the ask as a whole — judgedWords is that (novox/hq issue 868, review
+// round 1): Details is filtered line by line, not judged with the rest (holder.go's sayOn).
 func wholeAsk(r *sendRouter, id string) string {
 	a := r.published[id]
 	return a.Headline + "\n" + a.Explanation + "\n" + a.Details
 }
 
-// mustNotBeRefused fails the test naming what the content rule refused, and why.
-func mustNotBeRefused(t *testing.T, whole string) {
+// judgedWords is exactly the text messenger refuses (or lets through) an ask as a whole on (asks.go's
+// TakeAsk): its headline, its explanation, what happens at expiry, and each option's label and effect.
+// Details is never part of it.
+func judgedWords(a asks.Ask) string {
+	words := []string{a.Headline, a.Explanation, a.OnExpiry}
+	for _, o := range a.Options {
+		words = append(words, o.Label, o.Does)
+	}
+	return strings.Join(words, "\n")
+}
+
+// mustNotBeRefused fails the test naming what the content rule refused, and why — judged the way messenger
+// itself judges an ask as a whole, on judgedWords alone.
+func mustNotBeRefused(t *testing.T, a asks.Ask) {
 	t.Helper()
-	if refusal, ok := Check(whole); !ok {
-		t.Fatalf("the content rule refuses the ask: %s\n%s", refusal, whole)
+	if refusal, ok := Check(judgedWords(a)); !ok {
+		t.Fatalf("the content rule refuses the ask: %s\n%s", refusal, judgedWords(a))
 	}
 }
 
@@ -56,7 +72,7 @@ func TestMailSendAskForOwnAccountRecipientCarriesNoAddress(t *testing.T) {
 	if !strings.Contains(whole, "your account mesh") {
 		t.Errorf("the ask does not name the own account:\n%s", whole)
 	}
-	mustNotBeRefused(t, whole)
+	mustNotBeRefused(t, router.published[id])
 }
 
 // (2) A recipient outside the operator's own accounts is said without its address or host: its first
@@ -81,7 +97,7 @@ func TestMailSendAskForOutsideRecipientCarriesNoAddress(t *testing.T) {
 			t.Errorf("the ask does not say %q:\n%s", want, whole)
 		}
 	}
-	mustNotBeRefused(t, whole)
+	mustNotBeRefused(t, router.published[id])
 }
 
 // (3) More than three recipients are counted rather than said one by one.
@@ -106,7 +122,7 @@ func TestMailSendAskForManyRecipientsCountsTheRest(t *testing.T) {
 	if !strings.Contains(whole, "and 2 more") {
 		t.Errorf("the ask does not count the recipients past the first three:\n%s", whole)
 	}
-	mustNotBeRefused(t, whole)
+	mustNotBeRefused(t, router.published[id])
 }
 
 // (4) A subject holding a mail address and a link has both masked before the ask carries it.
@@ -131,5 +147,72 @@ func TestMailSendAskMasksAnAddressAndALinkInTheSubject(t *testing.T) {
 	if !strings.Contains(whole, "[address]") || !strings.Contains(whole, "[link]") {
 		t.Errorf("the ask does not say [address] and [link] in place of what it masked:\n%s", whole)
 	}
-	mustNotBeRefused(t, whole)
+	mustNotBeRefused(t, router.published[id])
+}
+
+// (5) review round 1: a subject the masking does not catch (an IP, a two-slash path-shaped reference, a
+// 40-character hex hash) does not refuse the whole ask: the real content rule (content_rule.go) still judges
+// the masked explanation as refused, so the subject and first lines are left out of it and pointed at Details
+// instead — the ask still names the account and recipients, and still reaches the operator.
+func TestMailSendAskFallsBackToDetailsWhenTheSubjectStillRefuses(t *testing.T) {
+	for _, subject := range []string{
+		"Invoice 2026/10/15",
+		"server 10.0.0.1",
+		"deadbeefdeadbeefdeadbeefdeadbeefdeadbeef", // 40 hex characters: reHex's shape
+	} {
+		t.Run(subject, func(t *testing.T) {
+			b, router, _ := sendTestBundle(t, Server{Host: "127.0.0.1", Port: 25, Security: "tls"})
+			rt := startBundle(t, b.Tools())
+			args := map[string]any{"account": "mesh", "to": []any{"ops@example.org"}, "subject": subject, "body": "body"}
+			text, isErr := rt.call(SendTool, "novox.triage", args)
+			if isErr {
+				t.Fatalf("%s", text)
+			}
+			var out map[string]any
+			_ = json.Unmarshal([]byte(text), &out)
+			id := out["ask"].(string)
+			a := router.published[id]
+			mustNotBeRefused(t, a)
+			if !strings.Contains(a.Explanation, "mesh") {
+				t.Errorf("the explanation does not name the account:\n%s", a.Explanation)
+			}
+			if !strings.Contains(a.Explanation, "o… at example org") {
+				t.Errorf("the explanation does not name the recipient:\n%s", a.Explanation)
+			}
+			if strings.Contains(a.Explanation, subject) {
+				t.Errorf("the explanation still carries the subject that refused the ask:\n%s", a.Explanation)
+			}
+			if !strings.Contains(a.Details, "Details") && !strings.Contains(a.Explanation, "Details") {
+				t.Errorf("the explanation does not point at Details:\n%s", a.Explanation)
+			}
+			if !strings.Contains(a.Details, subject) {
+				t.Errorf("Details does not carry the subject (filtered line by line by messenger, as the subject's own line): %s", a.Details)
+			}
+		})
+	}
+}
+
+// (6) review round 1: a recipient written with a display name ("Name <addr>") is matched against the
+// operator's own accounts, and masked, by its address alone.
+func TestMailSendAskMatchesADisplayNameRecipientByItsAddress(t *testing.T) {
+	b, router, _ := sendTestBundle(t, Server{Host: "127.0.0.1", Port: 25, Security: "tls"})
+	rt := startBundle(t, b.Tools())
+
+	args := map[string]any{"account": "mesh", "to": []any{"Jochen Schoubben <jschoubben@novox.be>"},
+		"subject": "Testing a display name", "body": "body"}
+	text, isErr := rt.call(SendTool, "novox.triage", args)
+	if isErr {
+		t.Fatalf("%s", text)
+	}
+	var out map[string]any
+	_ = json.Unmarshal([]byte(text), &out)
+	id := out["ask"].(string)
+	whole := wholeAsk(router, id)
+	if strings.Contains(whole, "@") || strings.Contains(whole, "Jochen Schoubben") {
+		t.Errorf("the ask carries the display-name recipient's address or name:\n%s", whole)
+	}
+	if !strings.Contains(whole, "your account mesh") {
+		t.Errorf("the ask does not match the display-name recipient to its own account:\n%s", whole)
+	}
+	mustNotBeRefused(t, router.published[id])
 }

modules/mesh-mail/cmd/mesh-mail/copies_test.go

@@ -1,10 +1,13 @@
 package main
 
-// content_test.go is messenger's own content rule (modules/messenger/cmd/messenger/content.go, novox/hq ADR
-// 0253 §3), copied here verbatim so mail_send's tests can judge its own asks against the real rule rather
-// than a guess at it (novox/hq issue 868). Messenger's content rule stays messenger's: this copy is never
-// built into the mesh-mail binary (it is a _test.go file, test-only), and this test pins it to the same
-// bytes as its source, the way modules/networkmanager/cmd/uplink-tools/copies_test.go pins uplink.go.
+// content_rule.go is messenger's own content rule (modules/messenger/cmd/messenger/content.go, novox/hq ADR
+// 0253 §3), copied here verbatim — byte for byte, only the file renamed, since Go allows nothing else to
+// differ (novox/hq issue 868, review round 1). mesh-mail carries it at run time, not only in its tests: the
+// ask it builds is checked against the real rule before it is published (send.go), so a subject or a first
+// line the masking does not catch (an IP, a hardware address, a path-shaped reference, a hex hash, …) falls
+// back to a safe explanation rather than refusing the whole ask. Messenger's content rule stays messenger's:
+// this is a pinned copy, never a dependency on it, the way modules/networkmanager/cmd/uplink-tools/copies_test.go
+// pins its copy of uplink.go.
 
 import (
 	"bytes"
@@ -19,12 +22,12 @@ func TestContentRuleCopyMatchesMessengers(t *testing.T) {
 	if err != nil {
 		t.Skipf("messenger's content.go is not beside this module (%v); nothing to pin against", err)
 	}
-	mine, err := os.ReadFile("content_test.go")
+	mine, err := os.ReadFile("content_rule.go")
 	if err != nil {
 		t.Fatal(err)
 	}
 	if !bytes.Equal(mine, theirs) {
-		t.Error("mesh-mail's copy of messenger's content.go (content_test.go) differs from " + src +
+		t.Error("mesh-mail's copy of messenger's content.go (content_rule.go) differs from " + src +
 			": change this copy to match messenger's content rule, verbatim, whenever it changes")
 	}
 }

modules/mesh-mail/cmd/mesh-mail/send.go

@@ -214,8 +214,14 @@ func firstChars(s string, n int) string {
 // Ask is the ask for this request, composed from it alone: the account, the recipients, the subject and the first
 // lines of the body (novox/hq ADR 0361 §2), in a form the operator channel's content rule (ADR 0253 §3) lets
 // through: AskTo names recipients without an address (composeAskTo, at AskMailSend time), and the subject and
-// body have every mail address and link or host name masked (maskText) before they reach the ask. The mail
-// itself is sent unchanged: neither masking touches s.To, s.Subject or s.Body (novox/hq issue 868).
+// body have every mail address and link or host name masked (maskText) before they reach the ask. Messenger
+// refuses an ask as a whole on its Headline, Explanation, OnExpiry and each option's Label and Does alone
+// (asks.go's TakeAsk) — never Details, which it filters line by line instead (holder.go's sayOn) — so once
+// those are composed, the real content rule (content_rule.go, mesh-mail's own run-time copy) judges them
+// exactly as messenger would. When the masking still leaves something the rule refuses (an IP, a hardware
+// address, a path-shaped reference, a hex hash, … novox/hq issue 868, review round 1), the subject and first
+// lines are left out of the Explanation and pointed at Details instead, so the ask always reaches the operator.
+// The mail itself is sent unchanged: neither masking touches s.To, s.Subject or s.Body.
 func (s SendRequest) Ask() asks.Ask {
 	to := s.AskTo
 	subject := maskText(s.Subject)
@@ -224,18 +230,34 @@ func (s SendRequest) Ask() asks.Ask {
 	if len([]rune(headline)) > asks.HeadlineLength {
 		headline = "Send mail?"
 	}
+	onExpiry := "nothing is sent"
+	options := []struct{ id, label, does string }{
+		{OptionSend, "Send", "the mail is sent through " + s.Account + "'s outgoing server"},
+		{OptionDecline, "Decline", "nothing is sent"},
+	}
+	// judged is exactly the words messenger refuses (or lets through) an ask as a whole on.
+	judged := func(explanation string) string {
+		words := []string{headline, explanation, onExpiry}
+		for _, o := range options {
+			words = append(words, o.label, o.does)
+		}
+		return strings.Join(words, "\n")
+	}
 	explanation := fmt.Sprintf("From %s to %s: %q.", s.Account, to, subject)
 	if len([]rune(explanation)) > asks.SummaryLength {
 		explanation = fmt.Sprintf("From %s: %q.", s.Account, subject)
 	}
+	if _, ok := Check(judged(explanation)); !ok {
+		explanation = fmt.Sprintf("From %s to %s. The subject and first lines are in Details.", s.Account, to)
+		if len([]rune(explanation)) > asks.SummaryLength {
+			explanation = "Send mail. The subject and first lines are in Details."
+		}
+	}
 	details := fmt.Sprintf("Asked by %s through the mesh-mail module, at %s UTC.\nAccount: %s\nTo: %s\nSubject: %s\n\n%s",
 		s.Caller, s.At.UTC().Format("15:04 on 2 Jan"), s.Account, to, subject, body)
 	a := asks.Ask{ID: s.ID, Headline: headline, Explanation: explanation, Details: details,
-		Who: asks.Operator, Expires: s.At.Add(SendAskLasts), OnExpiry: "nothing is sent"}
-	for _, o := range []struct{ id, label, does string }{
-		{OptionSend, "Send", "the mail is sent through " + s.Account + "'s outgoing server"},
-		{OptionDecline, "Decline", "nothing is sent"},
-	} {
+		Who: asks.Operator, Expires: s.At.Add(SendAskLasts), OnExpiry: onExpiry}
+	for _, o := range options {
 		binds, _ := asks.ActDigest(s.Act(o.id))
 		a.Options = append(a.Options, asks.Option{ID: o.id, Label: o.label, Does: o.does, Level: asks.Approve, Binds: binds})
 	}
@@ -454,12 +476,18 @@ func composeAskTo(to []string, accounts []Account) string {
 }
 
 // oneAskRecipient is one recipient's words in the ask: "your account <name>" for one of the accounts given, or
-// its first letter, an ellipsis and its domain's labels as words for any other ("j… at example com").
+// its first letter, an ellipsis and its domain's labels as words for any other ("j… at example com"). A
+// recipient written with a display name ("Name <addr>") is matched and masked by its address alone, parsed the
+// same way AskMailSend already validates it (novox/hq issue 868, review round 1).
 func oneAskRecipient(addr string, own map[string]string) string {
-	if name, ok := own[strings.ToLower(addr)]; ok {
+	bare := addr
+	if parsed, err := mail.ParseAddress(addr); err == nil {
+		bare = parsed.Address
+	}
+	if name, ok := own[strings.ToLower(bare)]; ok {
 		return "your account " + name
 	}
-	local, domain, found := strings.Cut(addr, "@")
+	local, domain, found := strings.Cut(bare, "@")
 	if !found || local == "" {
 		return "a recipient"
 	}