refactor: harden reset notice + cron delivery target flow

This commit is contained in:
Peter Steinberger
2026-02-23 19:00:30 +00:00
parent d266d12be1
commit bf373eeb43
6 changed files with 366 additions and 210 deletions

View File

@@ -382,8 +382,9 @@ describe("trigger handling", () => {
); );
const text = Array.isArray(res) ? res[0]?.text : res?.text; const text = Array.isArray(res) ? res[0]?.text : res?.text;
expect(text).toContain("api-key"); expect(text).toContain("api-key");
expect(text).toContain("****"); expect(text).toMatch(/\u2026|\.{3}/);
expect(text).toContain("sk-t"); expect(text).toContain("sk-tes");
expect(text).toContain("abcdef");
expect(text).not.toContain("1234567890abcdef"); expect(text).not.toContain("1234567890abcdef");
expect(text).toContain("(anthropic:work)"); expect(text).toContain("(anthropic:work)");
expect(text).not.toContain("mixed"); expect(text).not.toContain("mixed");

View File

@@ -51,6 +51,76 @@ import { appendUntrustedContext } from "./untrusted-context.js";
type AgentDefaults = NonNullable<OpenClawConfig["agents"]>["defaults"]; type AgentDefaults = NonNullable<OpenClawConfig["agents"]>["defaults"];
type ExecOverrides = Pick<ExecToolDefaults, "host" | "security" | "ask" | "node">; type ExecOverrides = Pick<ExecToolDefaults, "host" | "security" | "ask" | "node">;
function buildResetSessionNoticeText(params: {
provider: string;
model: string;
defaultProvider: string;
defaultModel: string;
}): string {
const modelLabel = `${params.provider}/${params.model}`;
const defaultLabel = `${params.defaultProvider}/${params.defaultModel}`;
return modelLabel === defaultLabel
? `✅ New session started · model: ${modelLabel}`
: `✅ New session started · model: ${modelLabel} (default: ${defaultLabel})`;
}
function resolveResetSessionNoticeRoute(params: {
ctx: MsgContext;
command: ReturnType<typeof buildCommandContext>;
}): {
channel: Parameters<typeof routeReply>[0]["channel"];
to: string;
} | null {
const commandChannel = params.command.channel?.trim().toLowerCase();
const fallbackChannel =
commandChannel && commandChannel !== "webchat"
? (commandChannel as Parameters<typeof routeReply>[0]["channel"])
: undefined;
const channel = params.ctx.OriginatingChannel ?? fallbackChannel;
const to = params.ctx.OriginatingTo ?? params.command.from ?? params.command.to;
if (!channel || channel === "webchat" || !to) {
return null;
}
return { channel, to };
}
async function sendResetSessionNotice(params: {
ctx: MsgContext;
command: ReturnType<typeof buildCommandContext>;
sessionKey: string;
cfg: OpenClawConfig;
accountId: string | undefined;
threadId: string | number | undefined;
provider: string;
model: string;
defaultProvider: string;
defaultModel: string;
}): Promise<void> {
const route = resolveResetSessionNoticeRoute({
ctx: params.ctx,
command: params.command,
});
if (!route) {
return;
}
await routeReply({
payload: {
text: buildResetSessionNoticeText({
provider: params.provider,
model: params.model,
defaultProvider: params.defaultProvider,
defaultModel: params.defaultModel,
}),
},
channel: route.channel,
to: route.to,
sessionKey: params.sessionKey,
accountId: params.accountId,
threadId: params.threadId,
cfg: params.cfg,
});
}
type RunPreparedReplyParams = { type RunPreparedReplyParams = {
ctx: MsgContext; ctx: MsgContext;
sessionCtx: TemplateContext; sessionCtx: TemplateContext;
@@ -318,27 +388,19 @@ export async function runPreparedReply(
} }
} }
if (resetTriggered && command.isAuthorizedSender) { if (resetTriggered && command.isAuthorizedSender) {
// oxlint-disable-next-line typescript/no-explicit-any await sendResetSessionNotice({
const channel = ctx.OriginatingChannel || (command.channel as any); ctx,
const to = ctx.OriginatingTo || command.from || command.to; command,
if (channel && to) {
const modelLabel = `${provider}/${model}`;
const defaultLabel = `${defaultProvider}/${defaultModel}`;
const text =
modelLabel === defaultLabel
? `✅ New session started · model: ${modelLabel}`
: `✅ New session started · model: ${modelLabel} (default: ${defaultLabel})`;
await routeReply({
payload: { text },
channel,
to,
sessionKey, sessionKey,
cfg,
accountId: ctx.AccountId, accountId: ctx.AccountId,
threadId: ctx.MessageThreadId, threadId: ctx.MessageThreadId,
cfg, provider,
model,
defaultProvider,
defaultModel,
}); });
} }
}
const sessionIdFinal = sessionId ?? crypto.randomUUID(); const sessionIdFinal = sessionId ?? crypto.randomUUID();
const sessionFile = resolveSessionFilePath( const sessionFile = resolveSessionFilePath(
sessionIdFinal, sessionIdFinal,

View File

@@ -74,6 +74,48 @@ describe("runCronIsolatedAgentTurn", () => {
setupIsolatedAgentTurnMocks({ fast: true }); setupIsolatedAgentTurnMocks({ fast: true });
}); });
it("does not fan out telegram cron delivery across allowFrom entries", async () => {
await withTempCronHome(async (home) => {
const { storePath, deps } = await createTelegramDeliveryFixture(home);
mockEmbeddedAgentPayloads([
{ text: "HEARTBEAT_OK", mediaUrl: "https://example.com/img.png" },
]);
const cfg = makeCfg(home, storePath, {
channels: {
telegram: {
botToken: "tok",
allowFrom: ["111", "222", "333"],
},
},
});
const res = await runCronIsolatedAgentTurn({
cfg,
deps,
job: {
...makeJob({
kind: "agentTurn",
message: "deliver once",
}),
delivery: { mode: "announce", channel: "telegram", to: "123" },
},
message: "deliver once",
sessionKey: "cron:job-1",
lane: "cron",
});
expect(res.status).toBe("ok");
expect(res.delivered).toBe(true);
expect(deps.sendMessageTelegram).toHaveBeenCalledTimes(1);
expect(deps.sendMessageTelegram).toHaveBeenCalledWith(
"123",
"HEARTBEAT_OK",
expect.objectContaining({ accountId: undefined }),
);
});
});
it("handles media heartbeat delivery and announce cleanup modes", async () => { it("handles media heartbeat delivery and announce cleanup modes", async () => {
await withTempCronHome(async (home) => { await withTempCronHome(async (home) => {
const { storePath, deps } = await createTelegramDeliveryFixture(home); const { storePath, deps } = await createTelegramDeliveryFixture(home);

View File

@@ -230,7 +230,11 @@ describe("resolveDeliveryTarget", () => {
target: { channel: "last", to: undefined }, target: { channel: "last", to: undefined },
}); });
expect(result.channel).toBe("telegram"); expect(result.channel).toBe("telegram");
expect(result.error).toBeUndefined(); expect(result.ok).toBe(false);
if (result.ok) {
throw new Error("expected unresolved delivery target");
}
expect(result.error.message).toContain('No delivery target resolved for channel "telegram"');
}); });
it("returns an error when channel selection is ambiguous", async () => { it("returns an error when channel selection is ambiguous", async () => {
@@ -245,7 +249,11 @@ describe("resolveDeliveryTarget", () => {
}); });
expect(result.channel).toBeUndefined(); expect(result.channel).toBeUndefined();
expect(result.to).toBeUndefined(); expect(result.to).toBeUndefined();
expect(result.error?.message).toContain("Channel is required"); expect(result.ok).toBe(false);
if (result.ok) {
throw new Error("expected ambiguous channel selection error");
}
expect(result.error.message).toContain("Channel is required");
}); });
it("uses sessionKey thread entry before main session entry", async () => { it("uses sessionKey thread entry before main session entry", async () => {
@@ -289,6 +297,6 @@ describe("resolveDeliveryTarget", () => {
expect(result.channel).toBe("telegram"); expect(result.channel).toBe("telegram");
expect(result.to).toBe("987654"); expect(result.to).toBe("987654");
expect(result.error).toBeUndefined(); expect(result.ok).toBe(true);
}); });
}); });

View File

@@ -17,6 +17,25 @@ import { normalizeAgentId } from "../../routing/session-key.js";
import { resolveWhatsAppAccount } from "../../web/accounts.js"; import { resolveWhatsAppAccount } from "../../web/accounts.js";
import { normalizeWhatsAppTarget } from "../../whatsapp/normalize.js"; import { normalizeWhatsAppTarget } from "../../whatsapp/normalize.js";
export type DeliveryTargetResolution =
| {
ok: true;
channel: Exclude<OutboundChannel, "none">;
to: string;
accountId?: string;
threadId?: string | number;
mode: "explicit" | "implicit";
}
| {
ok: false;
channel?: Exclude<OutboundChannel, "none">;
to?: string;
accountId?: string;
threadId?: string | number;
mode: "explicit" | "implicit";
error: Error;
};
export async function resolveDeliveryTarget( export async function resolveDeliveryTarget(
cfg: OpenClawConfig, cfg: OpenClawConfig,
agentId: string, agentId: string,
@@ -25,14 +44,7 @@ export async function resolveDeliveryTarget(
to?: string; to?: string;
sessionKey?: string; sessionKey?: string;
}, },
): Promise<{ ): Promise<DeliveryTargetResolution> {
channel?: Exclude<OutboundChannel, "none">;
to?: string;
accountId?: string;
threadId?: string | number;
mode: "explicit" | "implicit";
error?: Error;
}> {
const requestedChannel = typeof jobPayload.channel === "string" ? jobPayload.channel : "last"; const requestedChannel = typeof jobPayload.channel === "string" ? jobPayload.channel : "last";
const explicitTo = typeof jobPayload.to === "string" ? jobPayload.to : undefined; const explicitTo = typeof jobPayload.to === "string" ? jobPayload.to : undefined;
const allowMismatchedLastTo = requestedChannel === "last"; const allowMismatchedLastTo = requestedChannel === "last";
@@ -114,23 +126,29 @@ export async function resolveDeliveryTarget(
if (!channel) { if (!channel) {
return { return {
ok: false,
channel: undefined, channel: undefined,
to: undefined, to: undefined,
accountId, accountId,
threadId, threadId,
mode, mode,
error: channelResolutionError, error:
channelResolutionError ??
new Error("Channel is required when delivery.channel=last has no previous channel."),
}; };
} }
if (!toCandidate) { if (!toCandidate) {
return { return {
ok: false,
channel, channel,
to: undefined, to: undefined,
accountId, accountId,
threadId, threadId,
mode, mode,
error: channelResolutionError, error:
channelResolutionError ??
new Error(`No delivery target resolved for channel "${channel}". Set delivery.to.`),
}; };
} }
@@ -163,12 +181,23 @@ export async function resolveDeliveryTarget(
mode, mode,
allowFrom: allowFromOverride, allowFrom: allowFromOverride,
}); });
if (!docked.ok) {
return { return {
ok: false,
channel, channel,
to: docked.ok ? docked.to : undefined, to: undefined,
accountId,
threadId,
mode,
error: docked.error,
};
}
return {
ok: true,
channel,
to: docked.to,
accountId, accountId,
threadId, threadId,
mode, mode,
error: docked.ok ? channelResolutionError : docked.error,
}; };
} }

View File

@@ -635,6 +635,10 @@ export async function runCronIsolatedAgentTurn(params: {
// `true` means we confirmed at least one outbound send reached the target. // `true` means we confirmed at least one outbound send reached the target.
// Keep this strict so timer fallback can safely decide whether to wake main. // Keep this strict so timer fallback can safely decide whether to wake main.
let delivered = skipMessagingToolDelivery; let delivered = skipMessagingToolDelivery;
type SuccessfulDeliveryTarget = Extract<
Awaited<ReturnType<typeof resolveDeliveryTarget>>,
{ ok: true }
>;
const failDeliveryTarget = (error: string) => const failDeliveryTarget = (error: string) =>
withRunSession({ withRunSession({
status: "error", status: "error",
@@ -644,40 +648,10 @@ export async function runCronIsolatedAgentTurn(params: {
outputText, outputText,
...telemetry, ...telemetry,
}); });
if (deliveryRequested && !skipHeartbeatDelivery && !skipMessagingToolDelivery) { const deliverViaDirect = async (
if (resolvedDelivery.error) { delivery: SuccessfulDeliveryTarget,
if (!deliveryBestEffort) { ): Promise<RunCronAgentTurnResult | null> => {
return failDeliveryTarget(resolvedDelivery.error.message);
}
logWarn(`[cron:${params.job.id}] ${resolvedDelivery.error.message}`);
return withRunSession({ status: "ok", summary, outputText, ...telemetry });
}
const failOrWarnMissingDeliveryField = (message: string) => {
if (!deliveryBestEffort) {
return failDeliveryTarget(message);
}
logWarn(`[cron:${params.job.id}] ${message}`);
return withRunSession({ status: "ok", summary, outputText, ...telemetry });
};
if (!resolvedDelivery.channel) {
return failOrWarnMissingDeliveryField("cron delivery channel is missing");
}
if (!resolvedDelivery.to) {
return failOrWarnMissingDeliveryField("cron delivery target is missing");
}
const identity = resolveAgentOutboundIdentity(cfgWithAgentDefaults, agentId); const identity = resolveAgentOutboundIdentity(cfgWithAgentDefaults, agentId);
// Route text-only cron announce output back through the main session so it
// follows the same system-message injection path as subagent completions.
// Keep direct outbound delivery only for structured payloads (media/channel
// data), which cannot be represented by the shared announce flow.
//
// Forum/topic targets should also use direct delivery. Announce flow can
// be swallowed by ANNOUNCE_SKIP/NO_REPLY in the target agent turn, which
// silently drops cron output for topic-bound sessions.
const useDirectDelivery =
deliveryPayloadHasStructuredContent || resolvedDelivery.threadId != null;
if (useDirectDelivery) {
try { try {
const payloadsForDelivery = const payloadsForDelivery =
deliveryPayloads.length > 0 deliveryPayloads.length > 0
@@ -685,16 +659,18 @@ export async function runCronIsolatedAgentTurn(params: {
: synthesizedText : synthesizedText
? [{ text: synthesizedText }] ? [{ text: synthesizedText }]
: []; : [];
if (payloadsForDelivery.length > 0) { if (payloadsForDelivery.length === 0) {
return null;
}
if (isAborted()) { if (isAborted()) {
return withRunSession({ status: "error", error: abortReason(), ...telemetry }); return withRunSession({ status: "error", error: abortReason(), ...telemetry });
} }
const deliveryResults = await deliverOutboundPayloads({ const deliveryResults = await deliverOutboundPayloads({
cfg: cfgWithAgentDefaults, cfg: cfgWithAgentDefaults,
channel: resolvedDelivery.channel, channel: delivery.channel,
to: resolvedDelivery.to, to: delivery.to,
accountId: resolvedDelivery.accountId, accountId: delivery.accountId,
threadId: resolvedDelivery.threadId, threadId: delivery.threadId,
payloads: payloadsForDelivery, payloads: payloadsForDelivery,
agentId, agentId,
identity, identity,
@@ -703,7 +679,7 @@ export async function runCronIsolatedAgentTurn(params: {
abortSignal, abortSignal,
}); });
delivered = deliveryResults.length > 0; delivered = deliveryResults.length > 0;
} return null;
} catch (err) { } catch (err) {
if (!deliveryBestEffort) { if (!deliveryBestEffort) {
return withRunSession({ return withRunSession({
@@ -714,8 +690,15 @@ export async function runCronIsolatedAgentTurn(params: {
...telemetry, ...telemetry,
}); });
} }
return null;
}
};
const deliverViaAnnounce = async (
delivery: SuccessfulDeliveryTarget,
): Promise<RunCronAgentTurnResult | null> => {
if (!synthesizedText) {
return null;
} }
} else if (synthesizedText) {
const announceMainSessionKey = resolveAgentMainSessionKey({ const announceMainSessionKey = resolveAgentMainSessionKey({
cfg: params.cfg, cfg: params.cfg,
agentId, agentId,
@@ -725,10 +708,10 @@ export async function runCronIsolatedAgentTurn(params: {
agentId, agentId,
fallbackSessionKey: announceMainSessionKey, fallbackSessionKey: announceMainSessionKey,
delivery: { delivery: {
channel: resolvedDelivery.channel, channel: delivery.channel,
to: resolvedDelivery.to, to: delivery.to,
accountId: resolvedDelivery.accountId, accountId: delivery.accountId,
threadId: resolvedDelivery.threadId, threadId: delivery.threadId,
}, },
}); });
const taskLabel = const taskLabel =
@@ -791,10 +774,10 @@ export async function runCronIsolatedAgentTurn(params: {
childRunId: `${params.job.id}:${runSessionId}:${runStartedAt}`, childRunId: `${params.job.id}:${runSessionId}:${runStartedAt}`,
requesterSessionKey: announceSessionKey, requesterSessionKey: announceSessionKey,
requesterOrigin: { requesterOrigin: {
channel: resolvedDelivery.channel, channel: delivery.channel,
to: resolvedDelivery.to, to: delivery.to,
accountId: resolvedDelivery.accountId, accountId: delivery.accountId,
threadId: resolvedDelivery.threadId, threadId: delivery.threadId,
}, },
requesterDisplayKey: announceSessionKey, requesterDisplayKey: announceSessionKey,
task: taskLabel, task: taskLabel,
@@ -838,6 +821,37 @@ export async function runCronIsolatedAgentTurn(params: {
} }
logWarn(`[cron:${params.job.id}] ${String(err)}`); logWarn(`[cron:${params.job.id}] ${String(err)}`);
} }
return null;
};
if (deliveryRequested && !skipHeartbeatDelivery && !skipMessagingToolDelivery) {
if (!resolvedDelivery.ok) {
if (!deliveryBestEffort) {
return failDeliveryTarget(resolvedDelivery.error.message);
}
logWarn(`[cron:${params.job.id}] ${resolvedDelivery.error.message}`);
return withRunSession({ status: "ok", summary, outputText, ...telemetry });
}
// Route text-only cron announce output back through the main session so it
// follows the same system-message injection path as subagent completions.
// Keep direct outbound delivery only for structured payloads (media/channel
// data), which cannot be represented by the shared announce flow.
//
// Forum/topic targets should also use direct delivery. Announce flow can
// be swallowed by ANNOUNCE_SKIP/NO_REPLY in the target agent turn, which
// silently drops cron output for topic-bound sessions.
const useDirectDelivery =
deliveryPayloadHasStructuredContent || resolvedDelivery.threadId != null;
if (useDirectDelivery) {
const directResult = await deliverViaDirect(resolvedDelivery);
if (directResult) {
return directResult;
}
} else {
const announceResult = await deliverViaAnnounce(resolvedDelivery);
if (announceResult) {
return announceResult;
}
} }
} }