mesh-mail: mail_search's order defaults to descending when left out, matching its own schema text, and AC1's no-filter test proves ordering, limit and grant isolation together (issue 1232, review findings 5 and 7)

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

@@ -725,11 +725,13 @@ func orderByArg(args map[string]any) (string, error) {
 	return s, nil
 }
 
-// orderArg is 1230: asc or desc; anything else is refused (AC16), before the index is ever asked.
+// orderArg is 1230: asc or desc; desc (newest, largest, ...) when left out, as the schema says (review round 1
+// finding 5: this returned ascending on nothing given, contradicting its own description); anything else is
+// refused (AC16), before the index is ever asked.
 func orderArg(args map[string]any) (bool, error) {
 	v, ok := args["order"]
 	if !ok || v == nil {
-		return false, nil
+		return true, nil
 	}
 	s, ok := v.(string)
 	if !ok {

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

@@ -276,13 +276,58 @@ func TestARefusalSaysWhyTheGrantsCouldNotBeRead(t *testing.T) {
 
 // 1228/AC1, 1232: with no filter at all (not even query), mail_search answers the newest mails of the granted
 // accounts, not a refusal demanding a query.
+// 1228/AC1, 1232 (review round 1 finding 7): several mails over two granted accounts and folders, one mail of a
+// third account the caller is not granted, and a limit below the granted count — the newest granted mails
+// answer, in that exact order, never the ungranted account's, even though its mail is the newest of all.
 func TestMailSearchWithNoFilterAnswersTheNewestGrantedMail(t *testing.T) {
-	b, _, _ := aBundle(t)
+	dir := t.TempDir()
+	env := Env{Accounts: filepath.Join(dir, "accounts.json"), Grants: filepath.Join(dir, "grants.json"), Secrets: dir,
+		Status: filepath.Join(dir, "health.json")}
+	writeFile(t, env.Accounts, `{"accounts": {
+		"mesh": {"kind": "imap", "address": "jschoubben@novox.be", "incoming": {"host": "mail.novox.be", "port": 993, "security": "tls"}},
+		"gmail": {"kind": "imap", "address": "me@gmail.example", "incoming": {"host": "imap.gmail.com", "port": 993, "security": "tls"}},
+		"secret": {"kind": "imap", "address": "other@example.org", "incoming": {"host": "mail.example.org", "port": 993, "security": "tls"}}}}`)
+	writeFile(t, env.Grants, `{"grants": {"mesh": ["triage"], "gmail": ["triage"], "secret": ["other"]}}`)
+	st := newMemStore()
+	ctx := context.Background()
+	at := func(h int) time.Time { return judgedAt.Add(time.Duration(h) * time.Hour) }
+	for _, m := range []Message{
+		{Account: "mesh", Folder: "INBOX", UID: 1, MessageID: "<m1@mesh>", Subject: "m1", Date: at(1),
+			From: []Person{{Address: "a@example.org"}}, To: []Person{}, Flags: []string{}, Attachments: []AttachmentRef{}},
+		{Account: "mesh", Folder: "INBOX", UID: 2, MessageID: "<m2@mesh>", Subject: "m2", Date: at(2),
+			From: []Person{{Address: "a@example.org"}}, To: []Person{}, Flags: []string{}, Attachments: []AttachmentRef{}},
+		{Account: "mesh", Folder: "Sent", UID: 1, MessageID: "<m3@mesh>", Subject: "m3", Date: at(3),
+			From: []Person{{Address: "a@example.org"}}, To: []Person{}, Flags: []string{}, Attachments: []AttachmentRef{}},
+		{Account: "gmail", Folder: "INBOX", UID: 1, MessageID: "<m4@gmail>", Subject: "m4", Date: at(4),
+			From: []Person{{Address: "a@example.org"}}, To: []Person{}, Flags: []string{}, Attachments: []AttachmentRef{}},
+		{Account: "gmail", Folder: "Sent", UID: 1, MessageID: "<m5@gmail>", Subject: "m5", Date: at(5),
+			From: []Person{{Address: "a@example.org"}}, To: []Person{}, Flags: []string{}, Attachments: []AttachmentRef{}},
+		{Account: "mesh", Folder: "Junk", UID: 1, MessageID: "<m6@mesh>", Subject: "m6", Date: at(6),
+			From: []Person{{Address: "a@example.org"}}, To: []Person{}, Flags: []string{}, Attachments: []AttachmentRef{}},
+		// The newest of all, but in an account triage is not granted: it must never answer.
+		{Account: "secret", Folder: "INBOX", UID: 1, MessageID: "<s1@secret>", Subject: "s1", Date: at(7),
+			From: []Person{{Address: "a@example.org"}}, To: []Person{}, Flags: []string{}, Attachments: []AttachmentRef{}},
+	} {
+		if err := st.Add(ctx, m, nil); err != nil {
+			t.Fatal(err)
+		}
+	}
+	b := &Bundle{Env: env, Index: func(context.Context) (Store, error) { return st, nil }, Now: func() time.Time { return judgedAt }}
 	rt := startBundle(t, b.Tools())
-	text, isErr := rt.call(SearchTool, "novox.triage", map[string]any{})
-	if isErr || !strings.Contains(text, `"message-id":"att@example.org"`) {
+	text, isErr := rt.call(SearchTool, "novox.triage", map[string]any{"limit": 3})
+	if isErr {
 		t.Fatalf("%v %s", isErr, text)
 	}
+	if strings.Contains(text, "secret") || strings.Contains(text, "s1@secret") {
+		t.Fatalf("the ungranted account's mail answered: %s", text)
+	}
+	i6, i5, i4 := strings.Index(text, "m6@mesh"), strings.Index(text, "m5@gmail"), strings.Index(text, "m4@gmail")
+	if i6 < 0 || i5 < 0 || i4 < 0 || !(i6 < i5 && i5 < i4) {
+		t.Fatalf("not exactly m6, m5, m4 in that (newest-first) order: %s", text)
+	}
+	if strings.Contains(text, "m3@mesh") || strings.Contains(text, "m2@mesh") || strings.Contains(text, "m1@mesh") {
+		t.Fatalf("limit 3 answered more than the 3 newest: %s", text)
+	}
 }
 
 // 1232/AC16: an unknown order-by, a limit over 50, a date that does not parse, or an account not granted are
@@ -394,4 +439,15 @@ func TestMailSearchEndToEndFiltersAndGroupBySender(t *testing.T) {
 		grouped.Senders[0].Address != "alice@example.org" || grouped.Senders[0].Count != 2 {
 		t.Fatalf("%v %s", isErr, text)
 	}
+
+	// 1232/AC17 (review round 1 finding 5): order-by given, order left out, defaults to desc, as the schema's
+	// own description promises — not asc. "from alice" is a prefix of "from alice again", so ascending would put
+	// <a@mesh> first; descending puts <b@mesh> first.
+	text, isErr = rt.call(SearchTool, "novox.triage", map[string]any{"order-by": "subject"})
+	if isErr {
+		t.Fatalf("%v %s", isErr, text)
+	}
+	if first, second := strings.Index(text, "b@mesh"), strings.Index(text, "a@mesh"); first < 0 || second < 0 || first > second {
+		t.Fatalf("order-by with no order was not desc by default: %s", text)
+	}
 }