Skip to content

Commit bf373ee

Browse files
steipetesteipete
committed
refactor: harden reset notice + cron delivery target flow
1 parent d266d12 commit bf373ee

6 files changed

Lines changed: 365 additions & 209 deletions

‎src/auto-reply/reply.triggers.trigger-handling.filters-usage-summary-current-model-provider.test.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -382,8 +382,9 @@ describe("trigger handling", () => {
382382
);
383383
const text = Array.isArray(res) ? res[0]?.text : res?.text;
384384
expect(text).toContain("api-key");
385-
expect(text).toContain("****");
386-
expect(text).toContain("sk-t");
385+
expect(text).toMatch(/\u2026|\.{3}/);
386+
expect(text).toContain("sk-tes");
387+
expect(text).toContain("abcdef");
387388
expect(text).not.toContain("1234567890abcdef");
388389
expect(text).toContain("(anthropic:work)");
389390
expect(text).not.toContain("mixed");

‎src/auto-reply/reply/get-reply-run.ts‎

Lines changed: 82 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,76 @@ import { appendUntrustedContext } from "./untrusted-context.js";
5151
type AgentDefaults = NonNullable<OpenClawConfig["agents"]>["defaults"];
5252
type ExecOverrides = Pick<ExecToolDefaults, "host" | "security" | "ask" | "node">;
5353

54+
function buildResetSessionNoticeText(params: {
55+
provider: string;
56+
model: string;
57+
defaultProvider: string;
58+
defaultModel: string;
59+
}): string {
60+
const modelLabel = `${params.provider}/${params.model}`;
61+
const defaultLabel = `${params.defaultProvider}/${params.defaultModel}`;
62+
return modelLabel === defaultLabel
63+
? `✅ New session started · model: ${modelLabel}`
64+
: `✅ New session started · model: ${modelLabel} (default: ${defaultLabel})`;
65+
}
66+
67+
function resolveResetSessionNoticeRoute(params: {
68+
ctx: MsgContext;
69+
command: ReturnType<typeof buildCommandContext>;
70+
}): {
71+
channel: Parameters<typeof routeReply>[0]["channel"];
72+
to: string;
73+
} | null {
74+
const commandChannel = params.command.channel?.trim().toLowerCase();
75+
const fallbackChannel =
76+
commandChannel && commandChannel !== "webchat"
77+
? (commandChannel as Parameters<typeof routeReply>[0]["channel"])
78+
: undefined;
79+
const channel = params.ctx.OriginatingChannel ?? fallbackChannel;
80+
const to = params.ctx.OriginatingTo ?? params.command.from ?? params.command.to;
81+
if (!channel || channel === "webchat" || !to) {
82+
return null;
83+
}
84+
return { channel, to };
85+
}
86+
87+
async function sendResetSessionNotice(params: {
88+
ctx: MsgContext;
89+
command: ReturnType<typeof buildCommandContext>;
90+
sessionKey: string;
91+
cfg: OpenClawConfig;
92+
accountId: string | undefined;
93+
threadId: string | number | undefined;
94+
provider: string;
95+
model: string;
96+
defaultProvider: string;
97+
defaultModel: string;
98+
}): Promise<void> {
99+
const route = resolveResetSessionNoticeRoute({
100+
ctx: params.ctx,
101+
command: params.command,
102+
});
103+
if (!route) {
104+
return;
105+
}
106+
await routeReply({
107+
payload: {
108+
text: buildResetSessionNoticeText({
109+
provider: params.provider,
110+
model: params.model,
111+
defaultProvider: params.defaultProvider,
112+
defaultModel: params.defaultModel,
113+
}),
114+
},
115+
channel: route.channel,
116+
to: route.to,
117+
sessionKey: params.sessionKey,
118+
accountId: params.accountId,
119+
threadId: params.threadId,
120+
cfg: params.cfg,
121+
});
122+
}
123+
54124
type RunPreparedReplyParams = {
55125
ctx: MsgContext;
56126
sessionCtx: TemplateContext;
@@ -318,26 +388,18 @@ export async function runPreparedReply(
318388
}
319389
}
320390
if (resetTriggered && command.isAuthorizedSender) {
321-
// oxlint-disable-next-line typescript/no-explicit-any
322-
const channel = ctx.OriginatingChannel || (command.channel as any);
323-
const to = ctx.OriginatingTo || command.from || command.to;
324-
if (channel && to) {
325-
const modelLabel = `${provider}/${model}`;
326-
const defaultLabel = `${defaultProvider}/${defaultModel}`;
327-
const text =
328-
modelLabel === defaultLabel
329-
? `✅ New session started · model: ${modelLabel}`
330-
: `✅ New session started · model: ${modelLabel} (default: ${defaultLabel})`;
331-
await routeReply({
332-
payload: { text },
333-
channel,
334-
to,
335-
sessionKey,
336-
accountId: ctx.AccountId,
337-
threadId: ctx.MessageThreadId,
338-
cfg,
339-
});
340-
}
391+
await sendResetSessionNotice({
392+
ctx,
393+
command,
394+
sessionKey,
395+
cfg,
396+
accountId: ctx.AccountId,
397+
threadId: ctx.MessageThreadId,
398+
provider,
399+
model,
400+
defaultProvider,
401+
defaultModel,
402+
});
341403
}
342404
const sessionIdFinal = sessionId ?? crypto.randomUUID();
343405
const sessionFile = resolveSessionFilePath(

‎src/cron/isolated-agent.delivers-response-has-heartbeat-ok-but-includes.test.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,48 @@ describe("runCronIsolatedAgentTurn", () => {
7474
setupIsolatedAgentTurnMocks({ fast: true });
7575
});
7676

77+
it("does not fan out telegram cron delivery across allowFrom entries", async () => {
78+
await withTempCronHome(async (home) => {
79+
const { storePath, deps } = await createTelegramDeliveryFixture(home);
80+
mockEmbeddedAgentPayloads([
81+
{ text: "HEARTBEAT_OK", mediaUrl: "https://example.com/img.png" },
82+
]);
83+
84+
const cfg = makeCfg(home, storePath, {
85+
channels: {
86+
telegram: {
87+
botToken: "tok",
88+
allowFrom: ["111", "222", "333"],
89+
},
90+
},
91+
});
92+
93+
const res = await runCronIsolatedAgentTurn({
94+
cfg,
95+
deps,
96+
job: {
97+
...makeJob({
98+
kind: "agentTurn",
99+
message: "deliver once",
100+
}),
101+
delivery: { mode: "announce", channel: "telegram", to: "123" },
102+
},
103+
message: "deliver once",
104+
sessionKey: "cron:job-1",
105+
lane: "cron",
106+
});
107+
108+
expect(res.status).toBe("ok");
109+
expect(res.delivered).toBe(true);
110+
expect(deps.sendMessageTelegram).toHaveBeenCalledTimes(1);
111+
expect(deps.sendMessageTelegram).toHaveBeenCalledWith(
112+
"123",
113+
"HEARTBEAT_OK",
114+
expect.objectContaining({ accountId: undefined }),
115+
);
116+
});
117+
});
118+
77119
it("handles media heartbeat delivery and announce cleanup modes", async () => {
78120
await withTempCronHome(async (home) => {
79121
const { storePath, deps } = await createTelegramDeliveryFixture(home);

‎src/cron/isolated-agent/delivery-target.test.ts‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,11 @@ describe("resolveDeliveryTarget", () => {
230230
target: { channel: "last", to: undefined },
231231
});
232232
expect(result.channel).toBe("telegram");
233-
expect(result.error).toBeUndefined();
233+
expect(result.ok).toBe(false);
234+
if (result.ok) {
235+
throw new Error("expected unresolved delivery target");
236+
}
237+
expect(result.error.message).toContain('No delivery target resolved for channel "telegram"');
234238
});
235239

236240
it("returns an error when channel selection is ambiguous", async () => {
@@ -245,7 +249,11 @@ describe("resolveDeliveryTarget", () => {
245249
});
246250
expect(result.channel).toBeUndefined();
247251
expect(result.to).toBeUndefined();
248-
expect(result.error?.message).toContain("Channel is required");
252+
expect(result.ok).toBe(false);
253+
if (result.ok) {
254+
throw new Error("expected ambiguous channel selection error");
255+
}
256+
expect(result.error.message).toContain("Channel is required");
249257
});
250258

251259
it("uses sessionKey thread entry before main session entry", async () => {
@@ -289,6 +297,6 @@ describe("resolveDeliveryTarget", () => {
289297

290298
expect(result.channel).toBe("telegram");
291299
expect(result.to).toBe("987654");
292-
expect(result.error).toBeUndefined();
300+
expect(result.ok).toBe(true);
293301
});
294302
});

‎src/cron/isolated-agent/delivery-target.ts‎

Lines changed: 41 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,25 @@ import { normalizeAgentId } from "../../routing/session-key.js";
1717
import { resolveWhatsAppAccount } from "../../web/accounts.js";
1818
import { normalizeWhatsAppTarget } from "../../whatsapp/normalize.js";
1919

20+
export type DeliveryTargetResolution =
21+
| {
22+
ok: true;
23+
channel: Exclude<OutboundChannel, "none">;
24+
to: string;
25+
accountId?: string;
26+
threadId?: string | number;
27+
mode: "explicit" | "implicit";
28+
}
29+
| {
30+
ok: false;
31+
channel?: Exclude<OutboundChannel, "none">;
32+
to?: string;
33+
accountId?: string;
34+
threadId?: string | number;
35+
mode: "explicit" | "implicit";
36+
error: Error;
37+
};
38+
2039
export async function resolveDeliveryTarget(
2140
cfg: OpenClawConfig,
2241
agentId: string,
@@ -25,14 +44,7 @@ export async function resolveDeliveryTarget(
2544
to?: string;
2645
sessionKey?: string;
2746
},
28-
): Promise<{
29-
channel?: Exclude<OutboundChannel, "none">;
30-
to?: string;
31-
accountId?: string;
32-
threadId?: string | number;
33-
mode: "explicit" | "implicit";
34-
error?: Error;
35-
}> {
47+
): Promise<DeliveryTargetResolution> {
3648
const requestedChannel = typeof jobPayload.channel === "string" ? jobPayload.channel : "last";
3749
const explicitTo = typeof jobPayload.to === "string" ? jobPayload.to : undefined;
3850
const allowMismatchedLastTo = requestedChannel === "last";
@@ -114,23 +126,29 @@ export async function resolveDeliveryTarget(
114126

115127
if (!channel) {
116128
return {
129+
ok: false,
117130
channel: undefined,
118131
to: undefined,
119132
accountId,
120133
threadId,
121134
mode,
122-
error: channelResolutionError,
135+
error:
136+
channelResolutionError ??
137+
new Error("Channel is required when delivery.channel=last has no previous channel."),
123138
};
124139
}
125140

126141
if (!toCandidate) {
127142
return {
143+
ok: false,
128144
channel,
129145
to: undefined,
130146
accountId,
131147
threadId,
132148
mode,
133-
error: channelResolutionError,
149+
error:
150+
channelResolutionError ??
151+
new Error(`No delivery target resolved for channel "${channel}". Set delivery.to.`),
134152
};
135153
}
136154

@@ -163,12 +181,23 @@ export async function resolveDeliveryTarget(
163181
mode,
164182
allowFrom: allowFromOverride,
165183
});
184+
if (!docked.ok) {
185+
return {
186+
ok: false,
187+
channel,
188+
to: undefined,
189+
accountId,
190+
threadId,
191+
mode,
192+
error: docked.error,
193+
};
194+
}
166195
return {
196+
ok: true,
167197
channel,
168-
to: docked.ok ? docked.to : undefined,
198+
to: docked.to,
169199
accountId,
170200
threadId,
171201
mode,
172-
error: docked.ok ? channelResolutionError : docked.error,
173202
};
174203
}

0 commit comments

Comments
 (0)