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 <noreply@anthropic.com>
This commit is contained in:
scoootscooob
2026-03-02 14:39:03 -08:00
committed by Peter Steinberger
parent 55f04636f3
commit abb0252a1a
2 changed files with 132 additions and 1 deletions

View File

@@ -67,6 +67,15 @@ vi.mock("./queue.js", async () => {
};
});
const loadCronStoreMock = vi.fn();
vi.mock("../../cron/store.js", async () => {
const actual = await vi.importActual<typeof import("../../cron/store.js")>("../../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", () => {

View File

@@ -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<boolean> {
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;