mirror of https://github.com/openclaw/openclaw.git
fix: suppress NO_REPLY direct cron leaks (#45737) (thanks @openperf)
* fix(cron): suppress NO_REPLY sentinel in direct delivery path * fix: set deliveryAttempted on filtered NO_REPLY to prevent timer fallback * fix: mark silent NO_REPLY direct deliveries as delivered * fix(cron): unify silent direct delivery handling * fix: suppress NO_REPLY direct cron leaks (#45737) (thanks @openperf) --------- Co-authored-by: Ayaan Zaidi <hi@obviy.us>
This commit is contained in:
parent
08992e1dbc
commit
f463256660
|
|
@ -118,6 +118,7 @@ Docs: https://docs.openclaw.ai
|
|||
- Agents/subagents: honor allowlist validation, auth-profile handoff, and session override state when a subagent retries after `LiveSessionModelSwitchError`. (#58178) Thanks @openperf.
|
||||
- Agents/exec: restore `host=node` routing for node-pinned and `host=auto` sessions, while still blocking sandboxed `auto` sessions from jumping to gateway. (#60788) Thanks @openperf.
|
||||
- Agents/compaction: keep assistant tool calls and displaced tool results in the same compaction chunk so strict summarization providers stop rejecting orphaned tool pairs. (#58849) Thanks @openperf.
|
||||
- Cron: suppress exact `NO_REPLY` sentinel direct-delivery payloads, keep silent direct replies from falling back into duplicate main-summary sends, and treat structured `deleteAfterRun` silent replies the same as text silent replies. (#45737) Thanks @openperf.
|
||||
|
||||
## 2026.4.2
|
||||
|
||||
|
|
|
|||
|
|
@ -606,4 +606,98 @@ describe("dispatchCronDelivery — double-announce guard", () => {
|
|||
vi.unstubAllEnvs();
|
||||
}
|
||||
});
|
||||
|
||||
it("suppresses NO_REPLY payload in direct delivery so sentinel never leaks to external channels", async () => {
|
||||
vi.mocked(countActiveDescendantRuns).mockReturnValue(0);
|
||||
vi.mocked(isLikelyInterimCronMessage).mockReturnValue(false);
|
||||
|
||||
const params = makeBaseParams({ synthesizedText: "NO_REPLY" });
|
||||
// Force the useDirectDelivery path (structured content) to exercise
|
||||
// deliverViaDirect without going through finalizeTextDelivery.
|
||||
(params as Record<string, unknown>).deliveryPayloadHasStructuredContent = true;
|
||||
const state = await dispatchCronDelivery(params);
|
||||
|
||||
// NO_REPLY must be filtered out before reaching the outbound adapter.
|
||||
expect(deliverOutboundPayloads).not.toHaveBeenCalled();
|
||||
expect(state.result).toEqual(
|
||||
expect.objectContaining({
|
||||
status: "ok",
|
||||
delivered: false,
|
||||
deliveryAttempted: true,
|
||||
}),
|
||||
);
|
||||
// deliveryAttempted must be true so the heartbeat timer does not fire
|
||||
// a fallback enqueueSystemEvent with the NO_REPLY sentinel text.
|
||||
expect(state.deliveryAttempted).toBe(true);
|
||||
|
||||
// Verify timer guard agrees: shouldEnqueueCronMainSummary returns false
|
||||
expect(
|
||||
shouldEnqueueCronMainSummary({
|
||||
summaryText: "NO_REPLY",
|
||||
deliveryRequested: true,
|
||||
delivered: state.result?.delivered,
|
||||
deliveryAttempted: state.result?.deliveryAttempted,
|
||||
suppressMainSummary: false,
|
||||
isCronSystemEvent: () => true,
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("suppresses NO_REPLY payload with surrounding whitespace", async () => {
|
||||
vi.mocked(countActiveDescendantRuns).mockReturnValue(0);
|
||||
vi.mocked(isLikelyInterimCronMessage).mockReturnValue(false);
|
||||
|
||||
const params = makeBaseParams({ synthesizedText: " NO_REPLY " });
|
||||
(params as Record<string, unknown>).deliveryPayloadHasStructuredContent = true;
|
||||
const state = await dispatchCronDelivery(params);
|
||||
|
||||
expect(deliverOutboundPayloads).not.toHaveBeenCalled();
|
||||
expect(state.result).toEqual(
|
||||
expect.objectContaining({
|
||||
status: "ok",
|
||||
delivered: false,
|
||||
deliveryAttempted: true,
|
||||
}),
|
||||
);
|
||||
expect(state.deliveryAttempted).toBe(true);
|
||||
|
||||
expect(
|
||||
shouldEnqueueCronMainSummary({
|
||||
summaryText: " NO_REPLY ",
|
||||
deliveryRequested: true,
|
||||
delivered: state.result?.delivered,
|
||||
deliveryAttempted: state.result?.deliveryAttempted,
|
||||
suppressMainSummary: false,
|
||||
isCronSystemEvent: () => true,
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("cleans up the direct cron session after a structured silent reply when deleteAfterRun is enabled", async () => {
|
||||
vi.mocked(countActiveDescendantRuns).mockReturnValue(0);
|
||||
vi.mocked(isLikelyInterimCronMessage).mockReturnValue(false);
|
||||
|
||||
const params = makeBaseParams({ synthesizedText: SILENT_REPLY_TOKEN });
|
||||
(params as Record<string, unknown>).deliveryPayloadHasStructuredContent = true;
|
||||
(params.job as { deleteAfterRun?: boolean }).deleteAfterRun = true;
|
||||
|
||||
const state = await dispatchCronDelivery(params);
|
||||
|
||||
expect(state.result).toEqual(
|
||||
expect.objectContaining({
|
||||
status: "ok",
|
||||
delivered: false,
|
||||
deliveryAttempted: true,
|
||||
}),
|
||||
);
|
||||
expect(callGateway).toHaveBeenCalledWith({
|
||||
method: "sessions.delete",
|
||||
params: {
|
||||
key: "agent:main",
|
||||
deleteTranscript: true,
|
||||
emitLifecycleHooks: false,
|
||||
},
|
||||
timeoutMs: 10_000,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import { countActiveDescendantRuns } from "../../agents/subagent-registry-read.js";
|
||||
import { SILENT_REPLY_TOKEN } from "../../auto-reply/tokens.js";
|
||||
import { isSilentReplyText, SILENT_REPLY_TOKEN } from "../../auto-reply/tokens.js";
|
||||
import type { ReplyPayload } from "../../auto-reply/types.js";
|
||||
import type { CliDeps } from "../../cli/outbound-send-deps.js";
|
||||
import type { OpenClawConfig } from "../../config/config.js";
|
||||
|
|
@ -370,6 +370,37 @@ export async function dispatchCronDelivery(
|
|||
deliveryAttempted,
|
||||
...params.telemetry,
|
||||
});
|
||||
const cleanupDirectCronSessionIfNeeded = async (): Promise<void> => {
|
||||
if (!params.job.deleteAfterRun) {
|
||||
return;
|
||||
}
|
||||
try {
|
||||
const { callGateway } = await loadGatewayCallRuntime();
|
||||
await callGateway({
|
||||
method: "sessions.delete",
|
||||
params: {
|
||||
key: params.agentSessionKey,
|
||||
deleteTranscript: true,
|
||||
emitLifecycleHooks: false,
|
||||
},
|
||||
timeoutMs: 10_000,
|
||||
});
|
||||
} catch {
|
||||
// Best-effort; direct delivery result should still be returned.
|
||||
}
|
||||
};
|
||||
const finishSilentReplyDelivery = async (): Promise<RunCronAgentTurnResult> => {
|
||||
deliveryAttempted = true;
|
||||
await cleanupDirectCronSessionIfNeeded();
|
||||
return params.withRunSession({
|
||||
status: "ok",
|
||||
summary,
|
||||
outputText,
|
||||
delivered: false,
|
||||
deliveryAttempted: true,
|
||||
...params.telemetry,
|
||||
});
|
||||
};
|
||||
|
||||
const deliverViaDirect = async (
|
||||
delivery: SuccessfulDeliveryTarget,
|
||||
|
|
@ -387,14 +418,18 @@ export async function dispatchCronDelivery(
|
|||
delivery,
|
||||
});
|
||||
try {
|
||||
const payloadsForDelivery =
|
||||
const rawPayloads =
|
||||
deliveryPayloads.length > 0
|
||||
? deliveryPayloads
|
||||
: synthesizedText
|
||||
? [{ text: synthesizedText }]
|
||||
: [];
|
||||
// Suppress NO_REPLY sentinel so it never leaks to external channels.
|
||||
const payloadsForDelivery = rawPayloads.filter(
|
||||
(p) => !isSilentReplyText(p.text, SILENT_REPLY_TOKEN),
|
||||
);
|
||||
if (payloadsForDelivery.length === 0) {
|
||||
return null;
|
||||
return await finishSilentReplyDelivery();
|
||||
}
|
||||
if (params.isAborted()) {
|
||||
return params.withRunSession({
|
||||
|
|
@ -526,26 +561,6 @@ export async function dispatchCronDelivery(
|
|||
const finalizeTextDelivery = async (
|
||||
delivery: SuccessfulDeliveryTarget,
|
||||
): Promise<RunCronAgentTurnResult | null> => {
|
||||
const cleanupDirectCronSessionIfNeeded = async (): Promise<void> => {
|
||||
if (!params.job.deleteAfterRun) {
|
||||
return;
|
||||
}
|
||||
try {
|
||||
const { callGateway } = await loadGatewayCallRuntime();
|
||||
await callGateway({
|
||||
method: "sessions.delete",
|
||||
params: {
|
||||
key: params.agentSessionKey,
|
||||
deleteTranscript: true,
|
||||
emitLifecycleHooks: false,
|
||||
},
|
||||
timeoutMs: 10_000,
|
||||
});
|
||||
} catch {
|
||||
// Best-effort; direct delivery result should still be returned.
|
||||
}
|
||||
};
|
||||
|
||||
if (!synthesizedText) {
|
||||
return null;
|
||||
}
|
||||
|
|
@ -616,7 +631,7 @@ export async function dispatchCronDelivery(
|
|||
hadDescendants &&
|
||||
synthesizedText.trim() === initialSynthesizedText &&
|
||||
isLikelyInterimCronMessage(initialSynthesizedText) &&
|
||||
initialSynthesizedText.toUpperCase() !== SILENT_REPLY_TOKEN.toUpperCase()
|
||||
!isSilentReplyText(initialSynthesizedText, SILENT_REPLY_TOKEN)
|
||||
) {
|
||||
// Descendants existed but no post-orchestration synthesis arrived AND
|
||||
// no descendant fallback reply was available. Suppress stale parent
|
||||
|
|
@ -631,15 +646,8 @@ export async function dispatchCronDelivery(
|
|||
...params.telemetry,
|
||||
});
|
||||
}
|
||||
if (synthesizedText.toUpperCase() === SILENT_REPLY_TOKEN.toUpperCase()) {
|
||||
await cleanupDirectCronSessionIfNeeded();
|
||||
return params.withRunSession({
|
||||
status: "ok",
|
||||
summary,
|
||||
outputText,
|
||||
delivered: false,
|
||||
...params.telemetry,
|
||||
});
|
||||
if (isSilentReplyText(synthesizedText, SILENT_REPLY_TOKEN)) {
|
||||
return await finishSilentReplyDelivery();
|
||||
}
|
||||
if (params.isAborted()) {
|
||||
return params.withRunSession({
|
||||
|
|
|
|||
Loading…
Reference in New Issue