mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-12 03:33:43 +00:00
feat(cron): add --account flag for multi-account delivery routing (#26284)
* feat(cron): add --account flag for multi-account delivery routing Add support for explicit delivery account routing in cron jobs across CLI, normalization, delivery planning, and isolated delivery target resolution. Highlights: - Add --account <id> to cron add and cron edit - Add optional delivery.accountId to cron types and delivery plan - Normalize and trim delivery.accountId in cron create/update normalization - Prefer explicit accountId over session lastAccountId and bindings fallback - Thread accountId through isolated cron run delivery resolution - Preserve cron edit --best-effort-deliver/--no-best-effort-deliver behavior by keeping implicit announce mode - Expand tests for account passthrough/merge/precedence and CLI account flows * cron: resolve rebase duplicate accountId fields --------- Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com>
This commit is contained in:
@@ -43,6 +43,31 @@ describe("resolveCronDeliveryPlan", () => {
|
||||
expect(plan.requested).toBe(false);
|
||||
});
|
||||
|
||||
it("passes through accountId from delivery config", () => {
|
||||
const plan = resolveCronDeliveryPlan(
|
||||
makeJob({
|
||||
delivery: {
|
||||
mode: "announce",
|
||||
channel: "telegram",
|
||||
to: "-1003816714067",
|
||||
accountId: "coordinator",
|
||||
},
|
||||
}),
|
||||
);
|
||||
expect(plan.mode).toBe("announce");
|
||||
expect(plan.accountId).toBe("coordinator");
|
||||
expect(plan.to).toBe("-1003816714067");
|
||||
});
|
||||
|
||||
it("returns undefined accountId when not set", () => {
|
||||
const plan = resolveCronDeliveryPlan(
|
||||
makeJob({
|
||||
delivery: { mode: "announce", channel: "telegram", to: "123" },
|
||||
}),
|
||||
);
|
||||
expect(plan.accountId).toBeUndefined();
|
||||
});
|
||||
|
||||
it("resolves webhook mode without channel routing", () => {
|
||||
const plan = resolveCronDeliveryPlan(
|
||||
makeJob({
|
||||
|
||||
@@ -4,6 +4,7 @@ export type CronDeliveryPlan = {
|
||||
mode: CronDeliveryMode;
|
||||
channel?: CronMessageChannel;
|
||||
to?: string;
|
||||
/** Explicit channel account id from the delivery config, if set. */
|
||||
accountId?: string;
|
||||
source: "delivery" | "payload";
|
||||
requested: boolean;
|
||||
@@ -59,12 +60,11 @@ export function resolveCronDeliveryPlan(job: CronJob): CronDeliveryPlan {
|
||||
(delivery as { channel?: unknown } | undefined)?.channel,
|
||||
);
|
||||
const deliveryTo = normalizeTo((delivery as { to?: unknown } | undefined)?.to);
|
||||
const channel = deliveryChannel ?? payloadChannel ?? "last";
|
||||
const to = deliveryTo ?? payloadTo;
|
||||
const deliveryAccountId = normalizeAccountId(
|
||||
(delivery as { accountId?: unknown } | undefined)?.accountId,
|
||||
);
|
||||
|
||||
const channel = deliveryChannel ?? payloadChannel ?? "last";
|
||||
const to = deliveryTo ?? payloadTo;
|
||||
if (hasDelivery) {
|
||||
const resolvedMode = mode ?? "announce";
|
||||
return {
|
||||
|
||||
@@ -110,6 +110,27 @@ describe("resolveDeliveryTarget thread session lookup", () => {
|
||||
expect(result.channel).toBe("telegram");
|
||||
});
|
||||
|
||||
it("explicit accountId overrides session lastAccountId", async () => {
|
||||
mockStore["/mock/store.json"] = {
|
||||
"agent:main:main": {
|
||||
sessionId: "s1",
|
||||
updatedAt: 1,
|
||||
lastChannel: "telegram",
|
||||
lastTo: "-100444",
|
||||
lastAccountId: "session-account",
|
||||
},
|
||||
};
|
||||
|
||||
const result = await resolveDeliveryTarget(cfg, "main", {
|
||||
channel: "telegram",
|
||||
to: "-100444",
|
||||
accountId: "explicit-account",
|
||||
});
|
||||
|
||||
expect(result.accountId).toBe("explicit-account");
|
||||
expect(result.to).toBe("-100444");
|
||||
});
|
||||
|
||||
it("preserves threadId from :topic: when lastTo differs", async () => {
|
||||
mockStore["/mock/store.json"] = {
|
||||
"agent:main:main": {
|
||||
|
||||
@@ -42,8 +42,8 @@ export async function resolveDeliveryTarget(
|
||||
jobPayload: {
|
||||
channel?: "last" | ChannelId;
|
||||
to?: string;
|
||||
sessionKey?: string;
|
||||
accountId?: string;
|
||||
sessionKey?: string;
|
||||
},
|
||||
): Promise<DeliveryTargetResolution> {
|
||||
const requestedChannel = typeof jobPayload.channel === "string" ? jobPayload.channel : "last";
|
||||
@@ -101,11 +101,14 @@ export async function resolveDeliveryTarget(
|
||||
const mode = resolved.mode as "explicit" | "implicit";
|
||||
let toCandidate = resolved.to;
|
||||
|
||||
// When the session has no lastAccountId (e.g. first-run isolated cron
|
||||
// session), fall back to the agent's bound account from bindings config.
|
||||
// This ensures the message tool in isolated sessions resolves the correct
|
||||
// bot token for multi-account setups.
|
||||
let accountId = resolved.accountId;
|
||||
// Prefer an explicit accountId from the job's delivery config (set via
|
||||
// --account on cron add/edit). Fall back to the session's lastAccountId,
|
||||
// then to the agent's bound account from bindings config.
|
||||
const explicitAccountId =
|
||||
typeof jobPayload.accountId === "string" && jobPayload.accountId.trim()
|
||||
? jobPayload.accountId.trim()
|
||||
: undefined;
|
||||
let accountId = explicitAccountId ?? resolved.accountId;
|
||||
if (!accountId && channel) {
|
||||
const bindings = buildChannelAccountBindings(cfg);
|
||||
const byAgent = bindings.get(channel);
|
||||
@@ -115,11 +118,6 @@ export async function resolveDeliveryTarget(
|
||||
}
|
||||
}
|
||||
|
||||
// Explicit delivery account should override inferred session/binding account.
|
||||
if (jobPayload.accountId) {
|
||||
accountId = jobPayload.accountId;
|
||||
}
|
||||
|
||||
// Carry threadId when it was explicitly set (from :topic: parsing or config)
|
||||
// or when delivering to the same recipient as the session's last conversation.
|
||||
// Session-derived threadIds are dropped when the target differs to prevent
|
||||
|
||||
@@ -317,8 +317,8 @@ export async function runCronIsolatedAgentTurn(params: {
|
||||
const resolvedDelivery = await resolveDeliveryTarget(cfgWithAgentDefaults, agentId, {
|
||||
channel: deliveryPlan.channel ?? "last",
|
||||
to: deliveryPlan.to,
|
||||
sessionKey: params.job.sessionKey,
|
||||
accountId: deliveryPlan.accountId,
|
||||
sessionKey: params.job.sessionKey,
|
||||
});
|
||||
|
||||
const { formattedTime, timeLine } = resolveCronStyleNow(params.cfg, now);
|
||||
|
||||
@@ -212,6 +212,51 @@ describe("normalizeCronJobCreate", () => {
|
||||
expect(delivery.to).toBe("7200373102");
|
||||
});
|
||||
|
||||
it("normalizes delivery accountId and strips blanks", () => {
|
||||
const normalized = normalizeCronJobCreate({
|
||||
name: "delivery account",
|
||||
enabled: true,
|
||||
schedule: { kind: "cron", expr: "* * * * *" },
|
||||
sessionTarget: "isolated",
|
||||
wakeMode: "now",
|
||||
payload: {
|
||||
kind: "agentTurn",
|
||||
message: "hi",
|
||||
},
|
||||
delivery: {
|
||||
mode: "announce",
|
||||
channel: "telegram",
|
||||
to: "-1003816714067",
|
||||
accountId: " coordinator ",
|
||||
},
|
||||
}) as unknown as Record<string, unknown>;
|
||||
|
||||
const delivery = normalized.delivery as Record<string, unknown>;
|
||||
expect(delivery.accountId).toBe("coordinator");
|
||||
});
|
||||
|
||||
it("strips empty accountId from delivery", () => {
|
||||
const normalized = normalizeCronJobCreate({
|
||||
name: "empty account",
|
||||
enabled: true,
|
||||
schedule: { kind: "cron", expr: "* * * * *" },
|
||||
sessionTarget: "isolated",
|
||||
wakeMode: "now",
|
||||
payload: {
|
||||
kind: "agentTurn",
|
||||
message: "hi",
|
||||
},
|
||||
delivery: {
|
||||
mode: "announce",
|
||||
channel: "telegram",
|
||||
accountId: " ",
|
||||
},
|
||||
}) as unknown as Record<string, unknown>;
|
||||
|
||||
const delivery = normalized.delivery as Record<string, unknown>;
|
||||
expect("accountId" in delivery).toBe(false);
|
||||
});
|
||||
|
||||
it("normalizes webhook delivery mode and target URL", () => {
|
||||
const normalized = normalizeCronJobCreate({
|
||||
name: "webhook delivery",
|
||||
|
||||
@@ -183,6 +183,16 @@ function coerceDelivery(delivery: UnknownRecord) {
|
||||
delete next.to;
|
||||
}
|
||||
}
|
||||
if (typeof delivery.accountId === "string") {
|
||||
const trimmed = delivery.accountId.trim();
|
||||
if (trimmed) {
|
||||
next.accountId = trimmed;
|
||||
} else {
|
||||
delete next.accountId;
|
||||
}
|
||||
} else if ("accountId" in next && typeof next.accountId !== "string") {
|
||||
delete next.accountId;
|
||||
}
|
||||
return next;
|
||||
}
|
||||
|
||||
|
||||
@@ -115,6 +115,28 @@ describe("applyJobPatch", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("merges delivery.accountId from patch and preserves existing", () => {
|
||||
const job = createIsolatedAgentTurnJob("job-acct", {
|
||||
mode: "announce",
|
||||
channel: "telegram",
|
||||
to: "-100123",
|
||||
});
|
||||
|
||||
applyJobPatch(job, { delivery: { mode: "announce", accountId: " coordinator " } });
|
||||
expect(job.delivery?.accountId).toBe("coordinator");
|
||||
expect(job.delivery?.mode).toBe("announce");
|
||||
expect(job.delivery?.to).toBe("-100123");
|
||||
|
||||
// Updating other fields preserves accountId
|
||||
applyJobPatch(job, { delivery: { mode: "announce", to: "-100999" } });
|
||||
expect(job.delivery?.accountId).toBe("coordinator");
|
||||
expect(job.delivery?.to).toBe("-100999");
|
||||
|
||||
// Clearing accountId with empty string
|
||||
applyJobPatch(job, { delivery: { mode: "announce", accountId: "" } });
|
||||
expect(job.delivery?.accountId).toBeUndefined();
|
||||
});
|
||||
|
||||
it("rejects webhook delivery without a valid http(s) target URL", () => {
|
||||
const expectedError = "cron webhook delivery requires delivery.to to be a valid http(s) URL";
|
||||
const cases = [
|
||||
|
||||
@@ -609,6 +609,7 @@ function mergeCronDelivery(
|
||||
mode: existing?.mode ?? "none",
|
||||
channel: existing?.channel,
|
||||
to: existing?.to,
|
||||
accountId: existing?.accountId,
|
||||
bestEffort: existing?.bestEffort,
|
||||
};
|
||||
|
||||
@@ -623,6 +624,10 @@ function mergeCronDelivery(
|
||||
const to = typeof patch.to === "string" ? patch.to.trim() : "";
|
||||
next.to = to ? to : undefined;
|
||||
}
|
||||
if ("accountId" in patch) {
|
||||
const accountId = typeof patch.accountId === "string" ? patch.accountId.trim() : "";
|
||||
next.accountId = accountId ? accountId : undefined;
|
||||
}
|
||||
if (typeof patch.bestEffort === "boolean") {
|
||||
next.bestEffort = patch.bestEffort;
|
||||
}
|
||||
|
||||
@@ -22,6 +22,7 @@ export type CronDelivery = {
|
||||
mode: CronDeliveryMode;
|
||||
channel?: CronMessageChannel;
|
||||
to?: string;
|
||||
/** Explicit channel account id for multi-account setups (e.g. multiple Telegram bots). */
|
||||
accountId?: string;
|
||||
bestEffort?: boolean;
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user