From abb0252a1a4942199ae58a5b4a9ffe4bc77dea62 Mon Sep 17 00:00:00 2001 From: scoootscooob Date: Mon, 2 Mar 2026 14:39:03 -0800 Subject: [PATCH] fix(reply): suppress unscheduled-reminder note when session already has active cron Before appending the "I did not schedule a reminder" guard note, check the cron store for enabled jobs matching the current session key. This prevents false positives when the agent references an existing cron created in a prior turn (e.g. "I'll ping you when it's done" while a monitoring cron is already running). The check only fires on the rare path where the text matches commitment patterns AND no cron was added in the current turn, so the added I/O is negligible. Closes #32228 Co-Authored-By: Claude Opus 4.6 --- .../agent-runner.misc.runreplyagent.test.ts | 93 +++++++++++++++++++ src/auto-reply/reply/agent-runner.ts | 40 +++++++- 2 files changed, 132 insertions(+), 1 deletion(-) diff --git a/src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts b/src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts index 21e1d7682..bf0de740a 100644 --- a/src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts +++ b/src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts @@ -67,6 +67,15 @@ vi.mock("./queue.js", async () => { }; }); +const loadCronStoreMock = vi.fn(); +vi.mock("../../cron/store.js", async () => { + const actual = await vi.importActual("../../cron/store.js"); + return { + ...actual, + loadCronStore: (...args: unknown[]) => loadCronStoreMock(...args), + }; +}); + import { runReplyAgent } from "./agent-runner.js"; type RunWithModelFallbackParams = { @@ -80,6 +89,9 @@ beforeEach(() => { runCliAgentMock.mockClear(); runWithModelFallbackMock.mockClear(); runtimeErrorMock.mockClear(); + loadCronStoreMock.mockClear(); + // Default: no cron jobs in store. + loadCronStoreMock.mockResolvedValue({ version: 1, jobs: [] }); resetSystemEventsForTest(); // Default: no provider switch; execute the chosen provider+model. @@ -1180,6 +1192,87 @@ describe("runReplyAgent reminder commitment guard", () => { text: "I'll remind you tomorrow morning.", }); }); + + it("suppresses guard note when session already has an active cron job", async () => { + loadCronStoreMock.mockResolvedValueOnce({ + version: 1, + jobs: [ + { + id: "existing-job", + name: "monitor-task", + enabled: true, + sessionKey: "main", + createdAtMs: Date.now() - 60_000, + updatedAtMs: Date.now() - 60_000, + }, + ], + }); + + runEmbeddedPiAgentMock.mockResolvedValueOnce({ + payloads: [{ text: "I'll ping you when it's done." }], + meta: {}, + successfulCronAdds: 0, + }); + + const result = await createRun(); + expect(result).toMatchObject({ + text: "I'll ping you when it's done.", + }); + }); + + it("still appends guard note when cron jobs exist but not for the current session", async () => { + loadCronStoreMock.mockResolvedValueOnce({ + version: 1, + jobs: [ + { + id: "unrelated-job", + name: "daily-news", + enabled: true, + sessionKey: "other-session", + createdAtMs: Date.now() - 60_000, + updatedAtMs: Date.now() - 60_000, + }, + ], + }); + + runEmbeddedPiAgentMock.mockResolvedValueOnce({ + payloads: [{ text: "I'll remind you tomorrow morning." }], + meta: {}, + successfulCronAdds: 0, + }); + + const result = await createRun(); + expect(result).toMatchObject({ + text: "I'll remind you tomorrow morning.\n\nNote: I did not schedule a reminder in this turn, so this will not trigger automatically.", + }); + }); + + it("still appends guard note when cron jobs for session exist but are disabled", async () => { + loadCronStoreMock.mockResolvedValueOnce({ + version: 1, + jobs: [ + { + id: "disabled-job", + name: "old-monitor", + enabled: false, + sessionKey: "main", + createdAtMs: Date.now() - 60_000, + updatedAtMs: Date.now() - 60_000, + }, + ], + }); + + runEmbeddedPiAgentMock.mockResolvedValueOnce({ + payloads: [{ text: "I'll check back in an hour." }], + meta: {}, + successfulCronAdds: 0, + }); + + const result = await createRun(); + expect(result).toMatchObject({ + text: "I'll check back in an hour.\n\nNote: I did not schedule a reminder in this turn, so this will not trigger automatically.", + }); + }); }); describe("runReplyAgent fallback reasoning tags", () => { diff --git a/src/auto-reply/reply/agent-runner.ts b/src/auto-reply/reply/agent-runner.ts index a799fa9c6..0a49cf221 100644 --- a/src/auto-reply/reply/agent-runner.ts +++ b/src/auto-reply/reply/agent-runner.ts @@ -15,6 +15,7 @@ import { updateSessionStoreEntry, } from "../../config/sessions.js"; import type { TypingMode } from "../../config/types.js"; +import { loadCronStore, resolveCronStorePath } from "../../cron/store.js"; import { emitAgentEvent } from "../../infra/agent-events.js"; import { emitDiagnosticEvent, isDiagnosticsEnabled } from "../../infra/diagnostic-events.js"; import { generateSecureUuid } from "../../infra/secure-random.js"; @@ -71,6 +72,34 @@ function hasUnbackedReminderCommitment(text: string): boolean { return REMINDER_COMMITMENT_PATTERNS.some((pattern) => pattern.test(text)); } +/** + * Returns true when the cron store has at least one enabled job that shares the + * current session key. Used to suppress the "no reminder scheduled" guard note + * when an existing cron (created in a prior turn) already covers the commitment. + */ +async function hasSessionRelatedCronJobs(params: { + cronStorePath?: string; + sessionKey?: string; +}): Promise { + try { + const storePath = resolveCronStorePath(params.cronStorePath); + const store = await loadCronStore(storePath); + if (store.jobs.length === 0) { + return false; + } + // If we have a session key, only consider cron jobs from the same session. + // This avoids suppressing the note due to unrelated cron jobs. + if (params.sessionKey) { + return store.jobs.some((job) => job.enabled && job.sessionKey === params.sessionKey); + } + // Fallback: any enabled cron job counts. + return store.jobs.some((job) => job.enabled); + } catch { + // If we cannot read the cron store, do not suppress the note. + return false; + } +} + function appendUnscheduledReminderNote(payloads: ReplyPayload[]): ReplyPayload[] { let appended = false; return payloads.map((payload) => { @@ -540,8 +569,17 @@ export async function runReplyAgent(params: { typeof payload.text === "string" && hasUnbackedReminderCommitment(payload.text), ); - const guardedReplyPayloads = + // Suppress the guard note when an existing cron job (created in a prior + // turn) already covers the commitment — avoids false positives (#32228). + const coveredByExistingCron = hasReminderCommitment && successfulCronAdds === 0 + ? await hasSessionRelatedCronJobs({ + cronStorePath: cfg.cron?.store, + sessionKey, + }) + : false; + const guardedReplyPayloads = + hasReminderCommitment && successfulCronAdds === 0 && !coveredByExistingCron ? appendUnscheduledReminderNote(replyPayloads) : replyPayloads;