Skip to content

fix: prevent double draft emails when multi-rule selection enabled #1163

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
elie222 merged 5 commits into from
Jan 1, 2026
Merged

fix: prevent double draft emails when multi-rule selection enabled #1163

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2be0caf
feat(meeting-briefing): refactor to agentic approach with research tools
elie222 Jan 1, 2026
2de7d01
fixes
elie222 Jan 1, 2026
6ff89ca
adjust how we do research for meeting brief agent. simplify into a si…
elie222 Jan 1, 2026
bf92508
fix: prevent double draft emails when custom rule and TO_REPLY both h…
elie222 Jan 1, 2026
e1c7c88
fix: preserve skip reason when conversation rule resolves to nothing
elie222 Jan 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
492 changes: 492 additions & 0 deletions apps/web/__tests__/ai-meeting-briefing.test.ts
View file
Open in desktop

Large diffs are not rendered by default.

248 changes: 246 additions & 2 deletions apps/web/utils/ai/choose-rule/run-rules.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
ensureConversationRuleContinuity,
CONVERSATION_TRACKING_META_RULE_ID,
limitDraftEmailActions,
runRules,
} from "./run-rules";
import {
ActionType,
Expand All @@ -13,13 +14,37 @@ import type { Action } from "@/generated/prisma/client";
import { ConditionType } from "@/utils/config";
import prisma from "@/utils/__mocks__/prisma";
import type { RuleWithActions } from "@/utils/types";
import { getAction } from "@/__tests__/helpers";
import { getAction, getEmailAccount, getEmail } from "@/__tests__/helpers";
import { createScopedLogger } from "@/utils/logger";

const logger = createScopedLogger("test");

vi.mock("@/utils/prisma");
vi.mock("server-only", () => ({}));
vi.mock("next/server", () => ({ after: vi.fn((fn) => fn()) }));
vi.mock("@/utils/ai/choose-rule/match-rules", () => ({
findMatchingRules: vi.fn(),
}));
vi.mock("@/utils/reply-tracker/handle-conversation-status", () => ({
determineConversationStatus: vi.fn(),
updateThreadTrackers: vi.fn(),
}));
vi.mock("@/utils/ai/choose-rule/choose-args", () => ({
getActionItemsWithAiArgs: vi.fn(),
}));
vi.mock("@/utils/ai/choose-rule/execute", () => ({
executeAct: vi.fn(),
}));
vi.mock("@/utils/reply-tracker/label-helpers", () => ({
removeConflictingThreadStatusLabels: vi.fn(),
}));
vi.mock("@/utils/cold-email/is-cold-email", () => ({
saveColdEmail: vi.fn(),
}));
vi.mock("@/utils/scheduled-actions/scheduler", () => ({
scheduleDelayedActions: vi.fn(),
cancelScheduledActions: vi.fn(),
}));

const emailAccountId = "account-1";
const threadId = "thread-1";
Expand Down Expand Up @@ -424,9 +449,228 @@ describe("limitDraftEmailActions", () => {

const result = limitDraftEmailActions(matches, logger);

// Should select draft-2 because it has fixed content (static), even though draft-1 came first
expect(result[0].rule.actions).toEqual([]);
expect(result[1].rule.actions).toHaveLength(1);
expect(result[1].rule.actions[0].id).toBe("draft-2");
});

it("limits drafts when custom rule and resolved TO_REPLY both have DRAFT_EMAIL", () => {
const guestsRule = createRule("guests-rule", null, [
getAction({
id: "label-guest",
type: ActionType.LABEL,
label: "Guest Suggestion",
ruleId: "guests-rule",
}),
getAction({
id: "draft-guest",
type: ActionType.DRAFT_EMAIL,
content: "Hi {{name}}, Thank you for reaching out.",
ruleId: "guests-rule",
}),
]);

const toReplyRuleResolved = createRule(
"to-reply-resolved",
SystemType.TO_REPLY,
[
getAction({
id: "label-to-reply",
type: ActionType.LABEL,
label: "To Reply",
ruleId: "to-reply-resolved",
}),
getAction({
id: "draft-to-reply",
type: ActionType.DRAFT_EMAIL,
content: null,
ruleId: "to-reply-resolved",
}),
],
);

const resolvedMatches = [
{
rule: guestsRule,
matchReasons: undefined,
resolvedReason: undefined,
isConversationRule: false,
},
{
rule: toReplyRuleResolved,
matchReasons: undefined,
resolvedReason: "Needs reply",
isConversationRule: true,
},
];

const result = limitDraftEmailActions(resolvedMatches, logger);

expect(result[0].rule.actions).toHaveLength(2);
expect(
result[0].rule.actions.find((a) => a.type === ActionType.DRAFT_EMAIL)?.id,
).toBe("draft-guest");
expect(result[1].rule.actions).toHaveLength(1);
expect(result[1].rule.actions[0].type).toBe(ActionType.LABEL);

const typedResult = result as typeof resolvedMatches;
expect(typedResult[0].isConversationRule).toBe(false);
expect(typedResult[1].isConversationRule).toBe(true);
expect(typedResult[1].resolvedReason).toBe("Needs reply");
});

it("keeps first draft when both rules have AI-generated DRAFT_EMAIL", () => {
const guestsRule = createRule("guests-rule", null, [
getAction({
id: "draft-guest",
type: ActionType.DRAFT_EMAIL,
content: null,
ruleId: "guests-rule",
}),
]);

const toReplyRuleResolved = createRule(
"to-reply-resolved",
SystemType.TO_REPLY,
[
getAction({
id: "draft-to-reply",
type: ActionType.DRAFT_EMAIL,
content: null,
ruleId: "to-reply-resolved",
}),
],
);

const result = limitDraftEmailActions(
[{ rule: guestsRule }, { rule: toReplyRuleResolved }],
logger,
);

expect(result[0].rule.actions).toHaveLength(1);
expect(result[0].rule.actions[0].id).toBe("draft-guest");
expect(result[1].rule.actions).toEqual([]);
});
});

describe("runRules - double draft prevention", () => {
beforeEach(() => {
vi.clearAllMocks();
});

it("executes only one DRAFT_EMAIL when custom rule and TO_REPLY both have drafts", async () => {
const { findMatchingRules } = await import(
"@/utils/ai/choose-rule/match-rules"
);
const { determineConversationStatus } = await import(
"@/utils/reply-tracker/handle-conversation-status"
);
const { getActionItemsWithAiArgs } = await import(
"@/utils/ai/choose-rule/choose-args"
);
const { executeAct } = await import("@/utils/ai/choose-rule/execute");

const guestsRule = createRule("guests-rule", null, [
getAction({
id: "label-guest",
type: ActionType.LABEL,
label: "Guest Suggestion",
ruleId: "guests-rule",
}),
getAction({
id: "draft-guest",
type: ActionType.DRAFT_EMAIL,
content: "Hi {{name}}, Please submit via our form.",
ruleId: "guests-rule",
}),
]);

const metaRule = createRule(CONVERSATION_TRACKING_META_RULE_ID, null, []);

const toReplyWithDraft = createRule("to-reply-rule", SystemType.TO_REPLY, [
getAction({
id: "label-to-reply",
type: ActionType.LABEL,
label: "To Reply",
ruleId: "to-reply-rule",
}),
getAction({
id: "draft-to-reply",
type: ActionType.DRAFT_EMAIL,
content: null,
ruleId: "to-reply-rule",
}),
]);

vi.mocked(findMatchingRules).mockResolvedValue({
matches: [{ rule: guestsRule }, { rule: metaRule }],
reasoning: "Both rules matched",
});

vi.mocked(determineConversationStatus).mockResolvedValue({
rule: toReplyWithDraft,
reason: "Email needs a reply",
});

vi.mocked(getActionItemsWithAiArgs).mockImplementation(
async ({ selectedRule }) =>
selectedRule.actions.map((a) => ({ ...a, type: a.type as ActionType })),
);

const executedDraftContents: (string | null)[] = [];
vi.mocked(executeAct).mockImplementation(async ({ executedRule }) => {
for (const action of executedRule.actionItems) {
if (action.type === ActionType.DRAFT_EMAIL) {
executedDraftContents.push(action.content);
}
}
});

prisma.executedRule.findFirst.mockResolvedValue(null);

let createCallCount = 0;
(prisma.executedRule.create as any).mockImplementation(
async (args: any) => {
const actionItems = args.data.actionItems?.createMany?.data || [];
createCallCount++;
return {
id: `exec-${createCallCount}`,
status: ExecutedRuleStatus.APPLYING,
ruleId: args.data.rule?.connect?.id ?? null,
threadId: args.data.threadId,
messageId: args.data.messageId,
actionItems: actionItems.map((a: any, idx: number) => ({
...a,
id: a.id || `action-${createCallCount}-${idx}`,
executedRuleId: `exec-${createCallCount}`,
})),
};
},
);

const message = {
...getEmail(),
threadId,
snippet: "Test snippet",
historyId: "12345",
inline: [],
headers: { "message-id": "msg-1" },
attachments: [],
} as any;

await runRules({
provider: {} as any,
message,
rules: [guestsRule, toReplyWithDraft],
emailAccount: getEmailAccount(),
isTest: false,
modelType: "actionable" as any,
logger,
});

expect(executedDraftContents).toHaveLength(1);
expect(executedDraftContents[0]).toBe(
"Hi {{name}}, Please submit via our form.",
);
});
});
92 changes: 52 additions & 40 deletions apps/web/utils/ai/choose-rule/run-rules.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,15 +105,59 @@ export async function runRules({
logger,
});

const finalMatches = limitDraftEmailActions(conversationAwareMatches, logger);
// Separate regular matches from conversation meta-rule
const regularMatches = conversationAwareMatches.filter(
(m) => !isConversationRule(m.rule.id),
);
const conversationMatch = conversationAwareMatches.find((m) =>
isConversationRule(m.rule.id),
);

// Resolve conversation meta-rule to actual rule (e.g., TO_REPLY)
const matchesWithFlags: {
rule: RuleWithActions;
matchReasons?: MatchReason[];
resolvedReason?: string;
isConversationRule: boolean;
}[] = regularMatches.map((m) => ({
...m,
isConversationRule: false,
}));

let skippedConversationReason: string | undefined;

if (conversationMatch) {
const { rule, reason } = await determineConversationStatus({
conversationRules,
message,
emailAccount,
provider,
modelType,
Comment on lines +129 to +135
Copy link
Copy Markdown
Contributor

@baz-reviewer baz-reviewer Bot Jan 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the conversation meta-rule is present we now call determineConversationStatus and only push the resolved match into matchesWithFlags when rule is truthy. If determineConversationStatus returns undefined there is no branch that records the SKIPPED result with the provided reason, so the conversation match is silently dropped and, if there are no other matches, we fall through to the generic "No rules matched" branch without surfacing the actual conversation skip reason. Could we keep emitting the SKIPPED result/reason when the conversation rule resolves to nothing so we still know why nothing executed?

Finding type: Logical Bugs

Copy link
Copy Markdown
Contributor

@baz-reviewer baz-reviewer Bot Jan 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit e1c7c88 addressed this comment by introducing a skippedConversationReason variable that captures the reason when determineConversationStatus returns a falsy rule. The code now prioritizes this specific conversation skip reason over generic fallback messages when no matches are found, ensuring the actual conversation skip reason is surfaced rather than being silently dropped.

Copy link
Copy Markdown
Owner Author

@elie222 elie222 Jan 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e1c7c88. Now tracking skippedConversationReason when the conversation rule resolves to nothing, and using it in the SKIPPED result if no other matches exist.

isTest,
});
if (rule) {
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Jan 1, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: When determineConversationStatus returns no rule but provides a skip reason, that reason is silently discarded. Consider recording a SKIPPED result with the reason so debugging can surface why the conversation rule resolved to nothing instead of falling through to a generic "No rules matched" message.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/utils/ai/choose-rule/run-rules.ts, line 136:

<comment>When `determineConversationStatus` returns no rule but provides a skip reason, that reason is silently discarded. Consider recording a SKIPPED result with the reason so debugging can surface why the conversation rule resolved to nothing instead of falling through to a generic &quot;No rules matched&quot; message.</comment>

<file context>
@@ -105,7 +105,45 @@ export async function runRules({
+      modelType,
+      isTest,
+    });
+    if (rule) {
+      matchesWithFlags.push({
+        rule,
</file context>

✅ Addressed in e1c7c88

Copy link
Copy Markdown
Contributor

@baz-reviewer baz-reviewer Bot Jan 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit e1c7c88 addressed this comment by capturing the skip reason from determineConversationStatus in a new variable skippedConversationReason and prioritizing it when creating SKIPPED results. This ensures the specific reason why a conversation rule resolved to nothing is preserved for debugging instead of being silently discarded.

Copy link
Copy Markdown
Owner Author

@elie222 elie222 Jan 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e1c7c88. Added skippedConversationReason tracking - when the conversation rule resolves to nothing, we now preserve the skip reason and use it in the SKIPPED result.

matchesWithFlags.push({
rule,
matchReasons: conversationMatch.matchReasons,
resolvedReason: reason,
isConversationRule: true,
});
} else {
// Track why conversation rule was skipped (e.g., determined FYI but rule disabled)
skippedConversationReason = reason;
}
}

const finalMatches = limitDraftEmailActions(matchesWithFlags, logger);

logger.trace("Matching rule", () => ({
module: MODULE,
results: finalMatches.map(filterNullProperties),
}));

if (!finalMatches.length) {
const reason = results.reasoning || "No rules matched";
const reason =
skippedConversationReason || results.reasoning || "No rules matched";
if (!isTest) {
await prisma.executedRule.create({
data: {
Expand Down Expand Up @@ -141,35 +185,10 @@ export async function runRules({
const executedRules: RunRulesResult[] = [];
Copy link
Copy Markdown
Contributor

@macroscopeapp macroscopeapp Bot Jan 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When a conversation meta-rule matches but determineConversationStatus returns no rule, the skip isn’t recorded if other rules also matched. Consider adding a SKIPPED result (and DB record) for the conversation rule even when other rules apply, so the skip is captured consistently.

🚀 Want me to fix this? Reply ex: "fix it for me".

Copy link
Copy Markdown
Owner Author

@elie222 elie222 Jan 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not addressing this. When other rules match and execute, they get stored in the database - the user can see what happened. Recording a SKIPPED result for the conversation rule alongside executed rules would add noise. The skip reason is primarily useful when nothing at all matched, which is now handled with skippedConversationReason.


for (const result of finalMatches) {
let ruleToExecute = result.rule;
let reasonToUse = results.reasoning;

if (result.rule && isConversationRule(result.rule.id)) {
const { rule: statusRule, reason: statusReason } =
await determineConversationStatus({
conversationRules,
message,
emailAccount,
provider,
modelType,
isTest,
});

if (!statusRule) {
const executedRule: RunRulesResult = {
rule: null,
reason: statusReason || "No enabled conversation status rule found",
createdAt: batchTimestamp,
status: ExecutedRuleStatus.SKIPPED,
};
const ruleToExecute = result.rule;
const reasonToUse = result.resolvedReason || results.reasoning;

executedRules.push(executedRule);
continue;
}

ruleToExecute = statusRule;
reasonToUse = statusReason;
} else {
if (!result.isConversationRule) {
analyzeSenderPatternIfAiMatch({
isTest,
result,
Expand Down Expand Up @@ -571,16 +590,9 @@ function isConversationRule(ruleId: string): boolean {
* If there are no draft email actions, we return the matches as is.
* If there is only one draft email action, we return the matches as is.
*/
export function limitDraftEmailActions(
matches: {
rule: RuleWithActions;
matchReasons?: MatchReason[];
}[],
logger: Logger,
): {
rule: RuleWithActions;
matchReasons?: MatchReason[];
}[] {
export function limitDraftEmailActions<
T extends { rule: RuleWithActions; matchReasons?: MatchReason[] },
>(matches: T[], logger: Logger): T[] {
const draftCandidates = matches.flatMap((match) =>
match.rule.actions
.filter((action) => action.type === ActionType.DRAFT_EMAIL)
Expand Down
Loading
Loading