mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-08 02:11:23 +00:00
Subagents: restore announce chain + fix nested retry/drop regressions (#22223)
* Subagents: restore announce flow and fix nested delivery retries * fix: prep subagent announce + docs alignment (#22223) (thanks @tyler6204)
This commit is contained in:
@@ -4,11 +4,9 @@ import type { OpenClawConfig } from "../../config/config.js";
|
||||
vi.mock("../../config/sessions.js", () => ({
|
||||
loadSessionStore: vi.fn(),
|
||||
resolveStorePath: vi.fn().mockReturnValue("/tmp/test-store.json"),
|
||||
evaluateSessionFreshness: vi.fn().mockReturnValue({ fresh: true }),
|
||||
resolveSessionResetPolicy: vi.fn().mockReturnValue({ mode: "idle", idleMinutes: 60 }),
|
||||
}));
|
||||
|
||||
import { loadSessionStore, evaluateSessionFreshness } from "../../config/sessions.js";
|
||||
import { loadSessionStore } from "../../config/sessions.js";
|
||||
import { resolveCronSession } from "./session.js";
|
||||
|
||||
const NOW_MS = 1_737_600_000_000;
|
||||
@@ -17,25 +15,18 @@ type SessionStore = ReturnType<typeof loadSessionStore>;
|
||||
type SessionStoreEntry = SessionStore[string];
|
||||
type MockSessionStoreEntry = Partial<SessionStoreEntry>;
|
||||
|
||||
function resolveWithStoredEntry(params?: {
|
||||
sessionKey?: string;
|
||||
entry?: MockSessionStoreEntry;
|
||||
forceNew?: boolean;
|
||||
fresh?: boolean;
|
||||
}) {
|
||||
function resolveWithStoredEntry(params?: { sessionKey?: string; entry?: MockSessionStoreEntry }) {
|
||||
const sessionKey = params?.sessionKey ?? "webhook:stable-key";
|
||||
const store: SessionStore = params?.entry
|
||||
? ({ [sessionKey]: params.entry as SessionStoreEntry } as SessionStore)
|
||||
: {};
|
||||
vi.mocked(loadSessionStore).mockReturnValue(store);
|
||||
vi.mocked(evaluateSessionFreshness).mockReturnValue({ fresh: params?.fresh ?? true });
|
||||
|
||||
return resolveCronSession({
|
||||
cfg: {} as OpenClawConfig,
|
||||
sessionKey,
|
||||
agentId: "main",
|
||||
nowMs: NOW_MS,
|
||||
forceNew: params?.forceNew,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -85,76 +76,51 @@ describe("resolveCronSession", () => {
|
||||
expect(result.isNewSession).toBe(true);
|
||||
});
|
||||
|
||||
// New tests for session reuse behavior (#18027)
|
||||
describe("session reuse for webhooks/cron", () => {
|
||||
it("reuses existing sessionId when session is fresh", () => {
|
||||
const result = resolveWithStoredEntry({
|
||||
entry: {
|
||||
sessionId: "existing-session-id-123",
|
||||
updatedAt: NOW_MS - 1000,
|
||||
systemSent: true,
|
||||
},
|
||||
fresh: true,
|
||||
});
|
||||
|
||||
expect(result.sessionEntry.sessionId).toBe("existing-session-id-123");
|
||||
expect(result.isNewSession).toBe(false);
|
||||
expect(result.systemSent).toBe(true);
|
||||
it("always creates a new sessionId for cron/webhook runs", () => {
|
||||
const result = resolveWithStoredEntry({
|
||||
entry: {
|
||||
sessionId: "existing-session-id-123",
|
||||
updatedAt: NOW_MS - 1000,
|
||||
systemSent: true,
|
||||
},
|
||||
});
|
||||
|
||||
it("creates new sessionId when session is stale", () => {
|
||||
const result = resolveWithStoredEntry({
|
||||
entry: {
|
||||
sessionId: "old-session-id",
|
||||
updatedAt: NOW_MS - 86_400_000, // 1 day ago
|
||||
systemSent: true,
|
||||
modelOverride: "gpt-4.1-mini",
|
||||
providerOverride: "openai",
|
||||
sendPolicy: "allow",
|
||||
},
|
||||
fresh: false,
|
||||
});
|
||||
expect(result.sessionEntry.sessionId).not.toBe("existing-session-id-123");
|
||||
expect(result.isNewSession).toBe(true);
|
||||
expect(result.systemSent).toBe(false);
|
||||
});
|
||||
|
||||
expect(result.sessionEntry.sessionId).not.toBe("old-session-id");
|
||||
expect(result.isNewSession).toBe(true);
|
||||
expect(result.systemSent).toBe(false);
|
||||
expect(result.sessionEntry.modelOverride).toBe("gpt-4.1-mini");
|
||||
expect(result.sessionEntry.providerOverride).toBe("openai");
|
||||
expect(result.sessionEntry.sendPolicy).toBe("allow");
|
||||
it("preserves overrides while rolling a new sessionId", () => {
|
||||
const result = resolveWithStoredEntry({
|
||||
entry: {
|
||||
sessionId: "old-session-id",
|
||||
updatedAt: NOW_MS - 86_400_000,
|
||||
systemSent: true,
|
||||
modelOverride: "gpt-4.1-mini",
|
||||
providerOverride: "openai",
|
||||
sendPolicy: "allow",
|
||||
},
|
||||
});
|
||||
|
||||
it("creates new sessionId when forceNew is true", () => {
|
||||
const result = resolveWithStoredEntry({
|
||||
entry: {
|
||||
sessionId: "existing-session-id-456",
|
||||
updatedAt: NOW_MS - 1000,
|
||||
systemSent: true,
|
||||
modelOverride: "sonnet-4",
|
||||
providerOverride: "anthropic",
|
||||
},
|
||||
fresh: true,
|
||||
forceNew: true,
|
||||
});
|
||||
expect(result.sessionEntry.sessionId).not.toBe("old-session-id");
|
||||
expect(result.isNewSession).toBe(true);
|
||||
expect(result.systemSent).toBe(false);
|
||||
expect(result.sessionEntry.modelOverride).toBe("gpt-4.1-mini");
|
||||
expect(result.sessionEntry.providerOverride).toBe("openai");
|
||||
expect(result.sessionEntry.sendPolicy).toBe("allow");
|
||||
});
|
||||
|
||||
expect(result.sessionEntry.sessionId).not.toBe("existing-session-id-456");
|
||||
expect(result.isNewSession).toBe(true);
|
||||
expect(result.systemSent).toBe(false);
|
||||
expect(result.sessionEntry.modelOverride).toBe("sonnet-4");
|
||||
expect(result.sessionEntry.providerOverride).toBe("anthropic");
|
||||
it("creates new sessionId when entry exists but has no sessionId", () => {
|
||||
const result = resolveWithStoredEntry({
|
||||
entry: {
|
||||
updatedAt: NOW_MS - 1000,
|
||||
modelOverride: "some-model",
|
||||
},
|
||||
});
|
||||
|
||||
it("creates new sessionId when entry exists but has no sessionId", () => {
|
||||
const result = resolveWithStoredEntry({
|
||||
entry: {
|
||||
updatedAt: NOW_MS - 1000,
|
||||
modelOverride: "some-model",
|
||||
},
|
||||
});
|
||||
|
||||
expect(result.sessionEntry.sessionId).toBeDefined();
|
||||
expect(result.isNewSession).toBe(true);
|
||||
// Should still preserve other fields from entry
|
||||
expect(result.sessionEntry.modelOverride).toBe("some-model");
|
||||
});
|
||||
expect(result.sessionEntry.sessionId).toBeDefined();
|
||||
expect(result.isNewSession).toBe(true);
|
||||
// Should still preserve other fields from entry
|
||||
expect(result.sessionEntry.modelOverride).toBe("some-model");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,19 +1,12 @@
|
||||
import crypto from "node:crypto";
|
||||
import type { OpenClawConfig } from "../../config/config.js";
|
||||
import {
|
||||
evaluateSessionFreshness,
|
||||
loadSessionStore,
|
||||
resolveSessionResetPolicy,
|
||||
resolveStorePath,
|
||||
type SessionEntry,
|
||||
} from "../../config/sessions.js";
|
||||
import { loadSessionStore, resolveStorePath, type SessionEntry } from "../../config/sessions.js";
|
||||
|
||||
export function resolveCronSession(params: {
|
||||
cfg: OpenClawConfig;
|
||||
sessionKey: string;
|
||||
nowMs: number;
|
||||
agentId: string;
|
||||
forceNew?: boolean;
|
||||
}) {
|
||||
const sessionCfg = params.cfg.session;
|
||||
const storePath = resolveStorePath(sessionCfg?.store, {
|
||||
@@ -21,42 +14,8 @@ export function resolveCronSession(params: {
|
||||
});
|
||||
const store = loadSessionStore(storePath);
|
||||
const entry = store[params.sessionKey];
|
||||
|
||||
// Check if we can reuse an existing session
|
||||
let sessionId: string;
|
||||
let isNewSession: boolean;
|
||||
let systemSent: boolean;
|
||||
|
||||
if (!params.forceNew && entry?.sessionId) {
|
||||
// Evaluate freshness using the configured reset policy
|
||||
// Cron/webhook sessions use "direct" reset type (1:1 conversation style)
|
||||
const resetPolicy = resolveSessionResetPolicy({
|
||||
sessionCfg,
|
||||
resetType: "direct",
|
||||
});
|
||||
const freshness = evaluateSessionFreshness({
|
||||
updatedAt: entry.updatedAt,
|
||||
now: params.nowMs,
|
||||
policy: resetPolicy,
|
||||
});
|
||||
|
||||
if (freshness.fresh) {
|
||||
// Reuse existing session
|
||||
sessionId = entry.sessionId;
|
||||
isNewSession = false;
|
||||
systemSent = entry.systemSent ?? false;
|
||||
} else {
|
||||
// Session expired, create new
|
||||
sessionId = crypto.randomUUID();
|
||||
isNewSession = true;
|
||||
systemSent = false;
|
||||
}
|
||||
} else {
|
||||
// No existing session or forced new
|
||||
sessionId = crypto.randomUUID();
|
||||
isNewSession = true;
|
||||
systemSent = false;
|
||||
}
|
||||
const sessionId = crypto.randomUUID();
|
||||
const systemSent = false;
|
||||
|
||||
const sessionEntry: SessionEntry = {
|
||||
// Preserve existing per-session overrides even when rolling to a new sessionId.
|
||||
@@ -66,5 +25,5 @@ export function resolveCronSession(params: {
|
||||
updatedAt: params.nowMs,
|
||||
systemSent,
|
||||
};
|
||||
return { storePath, store, sessionEntry, systemSent, isNewSession };
|
||||
return { storePath, store, sessionEntry, systemSent, isNewSession: true };
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user