Эх сурвалжийг харах

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

The search-messages tool declared from, subject, isRead, and isFlagged in its
input schema, but the handler never forwarded them to searchMessages(), so they
were silently dropped -- passing from=X returned unfiltered results.

Wire all four through and build them into the AppleScript whose clause as AND
filters alongside the existing query (now parenthesized so OR grouping is
preserved when combined). Extract the clause builder into a pure, exported
buildSearchCondition() with unit tests, including the isRead:false /
isFlagged:false regression and quote/backslash escaping. Document from/subject
as substring matches in the schema.
Kevin May 1 сар өмнө
parent
commit
cc9649f541

+ 7 - 4
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");
     }

+ 22 - 1
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.
      *

Файлын зөрүү хэтэрхий том тул дарагдсан байна
+ 0 - 0
build/services/appleMailManager.d.ts.map


+ 36 - 20
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

+ 45 - 15
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 ---

+ 64 - 8
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

+ 66 - 0
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"');
+  });
+});

Энэ ялгаанд хэт олон файл өөрчлөгдсөн тул зарим файлыг харуулаагүй болно