Merge pull request #13 from scoutsolutions/fix/search-filters

fix(search): wire from/subject/isRead/isFlagged filters into search-messages

build/index.js

@@ -103,8 +103,11 @@ function withErrorHandling(handler, errorPrefix) {
 // --- search-messages ---
 server.tool("search-messages", {
     query: z.string().optional().describe("Text to search for in subject, sender, or content"),
-    from: z.string().optional().describe("Filter by sender email address"),
-    subject: z.string().optional().describe("Filter by subject line"),
+    from: z
+        .string()
+        .optional()
+        .describe("Filter by sender (substring match against the full sender string, i.e. display name + address — not an exact address match)"),
+    subject: z.string().optional().describe("Filter by subject line (substring match)"),
     mailbox: z
         .string()
         .optional()
@@ -115,8 +118,8 @@ server.tool("search-messages", {
     dateFrom: DATE_FILTER_SCHEMA.describe("Start date filter (e.g., 'January 1, 2026')"),
     dateTo: DATE_FILTER_SCHEMA.describe("End date filter (e.g., 'March 1, 2026')"),
     limit: z.number().optional().describe("Maximum number of results (default: 50)"),
-}, withErrorHandling(({ query, mailbox, account, limit = 50, dateFrom, dateTo }) => {
-    const messages = mailManager.searchMessages(query, mailbox, account, limit, dateFrom, dateTo);
+}, withErrorHandling(({ query, mailbox, account, limit = 50, dateFrom, dateTo, from, subject, isRead, isFlagged, }) => {
+    const messages = mailManager.searchMessages(query, mailbox, account, limit, dateFrom, dateTo, from, subject, isRead, isFlagged);
     if (messages.length === 0) {
         return successResponse("No messages found matching criteria");
     }

build/services/appleMailManager.d.ts

@@ -26,6 +26,27 @@ import type { Message, MessageContent, Mailbox, Account, Attachment, HealthCheck
  * execution via osascript. Error handling is consistent: methods
  * return null/false/empty-array on failure rather than throwing.
  */
+export interface SearchConditionFilters {
+    query?: string;
+    from?: string;
+    subject?: string;
+    isRead?: boolean;
+    isFlagged?: boolean;
+}
+/**
+ * Build the AppleScript `whose` clause for searchMessages from a filter set.
+ *
+ * - `query` is a subject-OR-sender substring match, parenthesized so it groups
+ *   correctly when ANDed with other filters.
+ * - `from` and `subject` are substring matches (`sender`/`subject` contains).
+ * - `isRead` / `isFlagged` are boolean status checks.
+ * - Returns "" when no filters are set. Every interpolated value is escaped.
+ *
+ * Exported for unit testing: the bug this addresses (filters declared in the
+ * tool schema but silently dropped) lived in this logic, so it gets direct
+ * coverage independent of Mail.app.
+ */
+export declare function buildSearchCondition(filters: SearchConditionFilters): string;
 export declare class AppleMailManager {
     /**
      * Default account used when no account is specified.
@@ -88,7 +109,7 @@ export declare class AppleMailManager {
      * @param limit - Maximum number of results
      * @returns Array of matching messages
      */
-    searchMessages(query?: string, mailbox?: string, account?: string, limit?: number, dateFrom?: string, dateTo?: string): Message[];
+    searchMessages(query?: string, mailbox?: string, account?: string, limit?: number, dateFrom?: string, dateTo?: string, from?: string, subject?: string, isRead?: boolean, isFlagged?: boolean): Message[];
     /**
      * Get a message by ID.
      *

build/services/appleMailManager.js

@@ -123,22 +123,40 @@ const MAILBOX_ALIASES = {
     junk: ["Junk", "Junk Email", "Spam", "JUNK", "junk"],
     archive: ["Archive", "ARCHIVE", "archive", "All Mail"],
 };
-// =============================================================================
-// Apple Mail Manager Class
-// =============================================================================
 /**
- * Manager class for Apple Mail operations.
+ * Build the AppleScript `whose` clause for searchMessages from a filter set.
  *
- * Provides methods for:
- * - Reading and searching messages
- * - Sending emails
- * - Managing mailboxes
- * - Listing accounts
+ * - `query` is a subject-OR-sender substring match, parenthesized so it groups
+ *   correctly when ANDed with other filters.
+ * - `from` and `subject` are substring matches (`sender`/`subject` contains).
+ * - `isRead` / `isFlagged` are boolean status checks.
+ * - Returns "" when no filters are set. Every interpolated value is escaped.
  *
- * All operations are synchronous since they rely on AppleScript
- * execution via osascript. Error handling is consistent: methods
- * return null/false/empty-array on failure rather than throwing.
+ * Exported for unit testing: the bug this addresses (filters declared in the
+ * tool schema but silently dropped) lived in this logic, so it gets direct
+ * coverage independent of Mail.app.
  */
+export function buildSearchCondition(filters) {
+    const { query, from, subject, isRead, isFlagged } = filters;
+    const conditions = [];
+    if (query) {
+        const safeQuery = escapeForAppleScript(query);
+        conditions.push(`(subject contains "${safeQuery}" or sender contains "${safeQuery}")`);
+    }
+    if (from) {
+        conditions.push(`sender contains "${escapeForAppleScript(from)}"`);
+    }
+    if (subject) {
+        conditions.push(`subject contains "${escapeForAppleScript(subject)}"`);
+    }
+    if (typeof isRead === "boolean") {
+        conditions.push(`read status is ${isRead ? "true" : "false"}`);
+    }
+    if (typeof isFlagged === "boolean") {
+        conditions.push(`flagged status is ${isFlagged ? "true" : "false"}`);
+    }
+    return conditions.length > 0 ? `whose ${conditions.join(" and ")}` : "";
+}
 export class AppleMailManager {
     /**
      * Default account used when no account is specified.
@@ -289,7 +307,7 @@ export class AppleMailManager {
      * @param limit - Maximum number of results
      * @returns Array of matching messages
      */
-    searchMessages(query, mailbox, account, limit = 50, dateFrom, dateTo) {
+    searchMessages(query, mailbox, account, limit = 50, dateFrom, dateTo, from, subject, isRead, isFlagged) {
         // If no account specified, search across all accounts
         if (!account) {
             const accounts = this.listAccounts();
@@ -298,18 +316,16 @@ export class AppleMailManager {
                 if (allMessages.length >= limit)
                     break;
                 const remaining = limit - allMessages.length;
-                const msgs = this.searchMessages(query, mailbox, acct.name, remaining, dateFrom, dateTo);
+                const msgs = this.searchMessages(query, mailbox, acct.name, remaining, dateFrom, dateTo, from, subject, isRead, isFlagged);
                 allMessages.push(...msgs);
             }
             return allMessages.slice(0, limit);
         }
         const targetAccount = this.resolveAccount(account);
-        // Build the search condition
-        let searchCondition = "";
-        if (query) {
-            const safeQuery = escapeForAppleScript(query);
-            searchCondition = `whose subject contains "${safeQuery}" or sender contains "${safeQuery}"`;
-        }
+        // `query` is a subject-OR-sender substring match; from/subject/isRead/isFlagged
+        // are additional AND filters. Date filtering stays post-fetch below — `whose`
+        // date comparisons are unreliable in Mail.app AppleScript. See buildSearchCondition.
+        const searchCondition = buildSearchCondition({ query, from, subject, isRead, isFlagged });
         // Build date filter AppleScript.
         // Note: dateFrom/dateTo are already validated by DATE_FILTER_SCHEMA (alphanumeric + safe
         // punctuation only), so escapeForAppleScript() below is belt-and-suspenders — it won't

src/index.ts

@@ -126,8 +126,13 @@ server.tool(
   "search-messages",
   {
     query: z.string().optional().describe("Text to search for in subject, sender, or content"),
-    from: z.string().optional().describe("Filter by sender email address"),
-    subject: z.string().optional().describe("Filter by subject line"),
+    from: z
+      .string()
+      .optional()
+      .describe(
+        "Filter by sender (substring match against the full sender string, i.e. display name + address — not an exact address match)"
+      ),
+    subject: z.string().optional().describe("Filter by subject line (substring match)"),
     mailbox: z
       .string()
       .optional()
@@ -139,22 +144,47 @@ server.tool(
     dateTo: DATE_FILTER_SCHEMA.describe("End date filter (e.g., 'March 1, 2026')"),
     limit: z.number().optional().describe("Maximum number of results (default: 50)"),
   },
-  withErrorHandling(({ query, mailbox, account, limit = 50, dateFrom, dateTo }) => {
-    const messages = mailManager.searchMessages(query, mailbox, account, limit, dateFrom, dateTo);
+  withErrorHandling(
+    ({
+      query,
+      mailbox,
+      account,
+      limit = 50,
+      dateFrom,
+      dateTo,
+      from,
+      subject,
+      isRead,
+      isFlagged,
+    }) => {
+      const messages = mailManager.searchMessages(
+        query,
+        mailbox,
+        account,
+        limit,
+        dateFrom,
+        dateTo,
+        from,
+        subject,
+        isRead,
+        isFlagged
+      );
 
-    if (messages.length === 0) {
-      return successResponse("No messages found matching criteria");
-    }
+      if (messages.length === 0) {
+        return successResponse("No messages found matching criteria");
+      }
 
-    const messageList = messages
-      .map(
-        (m) =>
-          `  - ID: ${m.id} | ${m.dateReceived.toLocaleDateString()} | ${m.subject} (from: ${m.sender}) [${m.isRead ? "read" : "unread"}]`
-      )
-      .join("\n");
+      const messageList = messages
+        .map(
+          (m) =>
+            `  - ID: ${m.id} | ${m.dateReceived.toLocaleDateString()} | ${m.subject} (from: ${m.sender}) [${m.isRead ? "read" : "unread"}]`
+        )
+        .join("\n");
 
-    return successResponse(`Found ${messages.length} message(s):\n${messageList}`);
-  }, "Error searching messages")
+      return successResponse(`Found ${messages.length} message(s):\n${messageList}`);
+    },
+    "Error searching messages"
+  )
 );
 
 // --- get-message ---

src/services/appleMailManager.ts

@@ -174,6 +174,49 @@ const MAILBOX_ALIASES: Record<string, string[]> = {
  * execution via osascript. Error handling is consistent: methods
  * return null/false/empty-array on failure rather than throwing.
  */
+export interface SearchConditionFilters {
+  query?: string;
+  from?: string;
+  subject?: string;
+  isRead?: boolean;
+  isFlagged?: boolean;
+}
+
+/**
+ * Build the AppleScript `whose` clause for searchMessages from a filter set.
+ *
+ * - `query` is a subject-OR-sender substring match, parenthesized so it groups
+ *   correctly when ANDed with other filters.
+ * - `from` and `subject` are substring matches (`sender`/`subject` contains).
+ * - `isRead` / `isFlagged` are boolean status checks.
+ * - Returns "" when no filters are set. Every interpolated value is escaped.
+ *
+ * Exported for unit testing: the bug this addresses (filters declared in the
+ * tool schema but silently dropped) lived in this logic, so it gets direct
+ * coverage independent of Mail.app.
+ */
+export function buildSearchCondition(filters: SearchConditionFilters): string {
+  const { query, from, subject, isRead, isFlagged } = filters;
+  const conditions: string[] = [];
+  if (query) {
+    const safeQuery = escapeForAppleScript(query);
+    conditions.push(`(subject contains "${safeQuery}" or sender contains "${safeQuery}")`);
+  }
+  if (from) {
+    conditions.push(`sender contains "${escapeForAppleScript(from)}"`);
+  }
+  if (subject) {
+    conditions.push(`subject contains "${escapeForAppleScript(subject)}"`);
+  }
+  if (typeof isRead === "boolean") {
+    conditions.push(`read status is ${isRead ? "true" : "false"}`);
+  }
+  if (typeof isFlagged === "boolean") {
+    conditions.push(`flagged status is ${isFlagged ? "true" : "false"}`);
+  }
+  return conditions.length > 0 ? `whose ${conditions.join(" and ")}` : "";
+}
+
 export class AppleMailManager {
   /**
    * Default account used when no account is specified.
@@ -350,7 +393,11 @@ export class AppleMailManager {
     account?: string,
     limit = 50,
     dateFrom?: string,
-    dateTo?: string
+    dateTo?: string,
+    from?: string,
+    subject?: string,
+    isRead?: boolean,
+    isFlagged?: boolean
   ): Message[] {
     // If no account specified, search across all accounts
     if (!account) {
@@ -359,7 +406,18 @@ export class AppleMailManager {
       for (const acct of accounts) {
         if (allMessages.length >= limit) break;
         const remaining = limit - allMessages.length;
-        const msgs = this.searchMessages(query, mailbox, acct.name, remaining, dateFrom, dateTo);
+        const msgs = this.searchMessages(
+          query,
+          mailbox,
+          acct.name,
+          remaining,
+          dateFrom,
+          dateTo,
+          from,
+          subject,
+          isRead,
+          isFlagged
+        );
         allMessages.push(...msgs);
       }
       return allMessages.slice(0, limit);
@@ -367,12 +425,10 @@ export class AppleMailManager {
 
     const targetAccount = this.resolveAccount(account);
 
-    // Build the search condition
-    let searchCondition = "";
-    if (query) {
-      const safeQuery = escapeForAppleScript(query);
-      searchCondition = `whose subject contains "${safeQuery}" or sender contains "${safeQuery}"`;
-    }
+    // `query` is a subject-OR-sender substring match; from/subject/isRead/isFlagged
+    // are additional AND filters. Date filtering stays post-fetch below — `whose`
+    // date comparisons are unreliable in Mail.app AppleScript. See buildSearchCondition.
+    const searchCondition = buildSearchCondition({ query, from, subject, isRead, isFlagged });
 
     // Build date filter AppleScript.
     // Note: dateFrom/dateTo are already validated by DATE_FILTER_SCHEMA (alphanumeric + safe

src/services/searchCondition.test.ts

@@ -0,0 +1,66 @@
+import { describe, it, expect } from "vitest";
+import { buildSearchCondition } from "@/services/appleMailManager.js";
+
+describe("buildSearchCondition", () => {
+  it("returns an empty string when no filters are set", () => {
+    expect(buildSearchCondition({})).toBe("");
+    expect(buildSearchCondition({ query: "" })).toBe("");
+  });
+
+  it("builds a parenthesized subject-OR-sender clause for query", () => {
+    expect(buildSearchCondition({ query: "invoice" })).toBe(
+      'whose (subject contains "invoice" or sender contains "invoice")'
+    );
+  });
+
+  it("maps `from` to a sender substring match", () => {
+    expect(buildSearchCondition({ from: "bob@example.com" })).toBe(
+      'whose sender contains "bob@example.com"'
+    );
+  });
+
+  it("maps `subject` to a subject substring match", () => {
+    expect(buildSearchCondition({ subject: "Q3 report" })).toBe(
+      'whose subject contains "Q3 report"'
+    );
+  });
+
+  // Regression: isRead:false / isFlagged:false must NOT be dropped. `typeof === "boolean"`
+  // guards against the falsy-value bug that a plain `if (isRead)` would reintroduce.
+  it("includes read status for both true and false", () => {
+    expect(buildSearchCondition({ isRead: true })).toBe("whose read status is true");
+    expect(buildSearchCondition({ isRead: false })).toBe("whose read status is false");
+  });
+
+  it("includes flagged status for both true and false", () => {
+    expect(buildSearchCondition({ isFlagged: true })).toBe("whose flagged status is true");
+    expect(buildSearchCondition({ isFlagged: false })).toBe("whose flagged status is false");
+  });
+
+  // The whole point of the parentheses: when query is ANDed with other filters,
+  // the OR must stay grouped or precedence flips the meaning.
+  it("preserves OR grouping when combined with AND filters", () => {
+    expect(buildSearchCondition({ query: "x", from: "y@z.com", isRead: false })).toBe(
+      'whose (subject contains "x" or sender contains "x") and sender contains "y@z.com" and read status is false'
+    );
+  });
+
+  it("joins all filters with `and` in a stable order", () => {
+    expect(
+      buildSearchCondition({
+        query: "q",
+        from: "f",
+        subject: "s",
+        isRead: true,
+        isFlagged: false,
+      })
+    ).toBe(
+      'whose (subject contains "q" or sender contains "q") and sender contains "f" and subject contains "s" and read status is true and flagged status is false'
+    );
+  });
+
+  it("escapes embedded quotes and backslashes in interpolated values", () => {
+    expect(buildSearchCondition({ from: 'a"b' })).toBe('whose sender contains "a\\"b"');
+    expect(buildSearchCondition({ subject: "a\\b" })).toBe('whose subject contains "a\\\\b"');
+  });
+});