mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-08 12:11:23 +00:00
fix(cron): reuse existing sessionId for webhook/cron sessions
When a webhook or cron job provides a stable sessionKey, the session should maintain conversation history across invocations. Previously, resolveCronSession always generated a new sessionId and hardcoded isNewSession: true, preventing any conversation continuity. Changes: - Check if existing entry has a valid sessionId - Evaluate freshness using configured reset policy - Reuse sessionId and set isNewSession: false when fresh - Add forceNew parameter to override reuse behavior - Spread existing entry to preserve conversation context This enables persistent, stateful conversations for webhook-driven agent endpoints when allowRequestSessionKey is configured. Fixes #18027
This commit is contained in:
committed by
Peter Steinberger
parent
952db1a3e2
commit
57c8f62396
@@ -4,9 +4,14 @@ import type { OpenClawConfig } from "../../config/config.js";
|
|||||||
vi.mock("../../config/sessions.js", () => ({
|
vi.mock("../../config/sessions.js", () => ({
|
||||||
loadSessionStore: vi.fn(),
|
loadSessionStore: vi.fn(),
|
||||||
resolveStorePath: vi.fn().mockReturnValue("/tmp/test-store.json"),
|
resolveStorePath: vi.fn().mockReturnValue("/tmp/test-store.json"),
|
||||||
|
evaluateSessionFreshness: vi.fn().mockReturnValue({ fresh: true }),
|
||||||
|
resolveSessionResetPolicy: vi.fn().mockReturnValue({ mode: "idle", idleMinutes: 60 }),
|
||||||
}));
|
}));
|
||||||
|
|
||||||
import { loadSessionStore } from "../../config/sessions.js";
|
import {
|
||||||
|
loadSessionStore,
|
||||||
|
evaluateSessionFreshness,
|
||||||
|
} from "../../config/sessions.js";
|
||||||
import { resolveCronSession } from "./session.js";
|
import { resolveCronSession } from "./session.js";
|
||||||
|
|
||||||
describe("resolveCronSession", () => {
|
describe("resolveCronSession", () => {
|
||||||
@@ -21,6 +26,7 @@ describe("resolveCronSession", () => {
|
|||||||
model: "k2p5",
|
model: "k2p5",
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
|
vi.mocked(evaluateSessionFreshness).mockReturnValue({ fresh: true });
|
||||||
|
|
||||||
const result = resolveCronSession({
|
const result = resolveCronSession({
|
||||||
cfg: {} as OpenClawConfig,
|
cfg: {} as OpenClawConfig,
|
||||||
@@ -44,6 +50,7 @@ describe("resolveCronSession", () => {
|
|||||||
model: "claude-opus-4-5",
|
model: "claude-opus-4-5",
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
|
vi.mocked(evaluateSessionFreshness).mockReturnValue({ fresh: true });
|
||||||
|
|
||||||
const result = resolveCronSession({
|
const result = resolveCronSession({
|
||||||
cfg: {} as OpenClawConfig,
|
cfg: {} as OpenClawConfig,
|
||||||
@@ -69,5 +76,98 @@ describe("resolveCronSession", () => {
|
|||||||
expect(result.sessionEntry.modelOverride).toBeUndefined();
|
expect(result.sessionEntry.modelOverride).toBeUndefined();
|
||||||
expect(result.sessionEntry.providerOverride).toBeUndefined();
|
expect(result.sessionEntry.providerOverride).toBeUndefined();
|
||||||
expect(result.sessionEntry.model).toBeUndefined();
|
expect(result.sessionEntry.model).toBeUndefined();
|
||||||
|
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", () => {
|
||||||
|
vi.mocked(loadSessionStore).mockReturnValue({
|
||||||
|
"webhook:stable-key": {
|
||||||
|
sessionId: "existing-session-id-123",
|
||||||
|
updatedAt: Date.now() - 1000,
|
||||||
|
systemSent: true,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
vi.mocked(evaluateSessionFreshness).mockReturnValue({ fresh: true });
|
||||||
|
|
||||||
|
const result = resolveCronSession({
|
||||||
|
cfg: {} as OpenClawConfig,
|
||||||
|
sessionKey: "webhook:stable-key",
|
||||||
|
agentId: "main",
|
||||||
|
nowMs: Date.now(),
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(result.sessionEntry.sessionId).toBe("existing-session-id-123");
|
||||||
|
expect(result.isNewSession).toBe(false);
|
||||||
|
expect(result.systemSent).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("creates new sessionId when session is stale", () => {
|
||||||
|
vi.mocked(loadSessionStore).mockReturnValue({
|
||||||
|
"webhook:stable-key": {
|
||||||
|
sessionId: "old-session-id",
|
||||||
|
updatedAt: Date.now() - 86400000, // 1 day ago
|
||||||
|
systemSent: true,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
vi.mocked(evaluateSessionFreshness).mockReturnValue({ fresh: false });
|
||||||
|
|
||||||
|
const result = resolveCronSession({
|
||||||
|
cfg: {} as OpenClawConfig,
|
||||||
|
sessionKey: "webhook:stable-key",
|
||||||
|
agentId: "main",
|
||||||
|
nowMs: Date.now(),
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(result.sessionEntry.sessionId).not.toBe("old-session-id");
|
||||||
|
expect(result.isNewSession).toBe(true);
|
||||||
|
expect(result.systemSent).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("creates new sessionId when forceNew is true", () => {
|
||||||
|
vi.mocked(loadSessionStore).mockReturnValue({
|
||||||
|
"webhook:stable-key": {
|
||||||
|
sessionId: "existing-session-id-456",
|
||||||
|
updatedAt: Date.now() - 1000,
|
||||||
|
systemSent: true,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
vi.mocked(evaluateSessionFreshness).mockReturnValue({ fresh: true });
|
||||||
|
|
||||||
|
const result = resolveCronSession({
|
||||||
|
cfg: {} as OpenClawConfig,
|
||||||
|
sessionKey: "webhook:stable-key",
|
||||||
|
agentId: "main",
|
||||||
|
nowMs: Date.now(),
|
||||||
|
forceNew: true,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(result.sessionEntry.sessionId).not.toBe("existing-session-id-456");
|
||||||
|
expect(result.isNewSession).toBe(true);
|
||||||
|
expect(result.systemSent).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("creates new sessionId when entry exists but has no sessionId", () => {
|
||||||
|
vi.mocked(loadSessionStore).mockReturnValue({
|
||||||
|
"webhook:stable-key": {
|
||||||
|
updatedAt: Date.now() - 1000,
|
||||||
|
modelOverride: "some-model",
|
||||||
|
} as any,
|
||||||
|
});
|
||||||
|
vi.mocked(evaluateSessionFreshness).mockReturnValue({ fresh: true });
|
||||||
|
|
||||||
|
const result = resolveCronSession({
|
||||||
|
cfg: {} as OpenClawConfig,
|
||||||
|
sessionKey: "webhook:stable-key",
|
||||||
|
agentId: "main",
|
||||||
|
nowMs: Date.now(),
|
||||||
|
});
|
||||||
|
|
||||||
|
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,12 +1,19 @@
|
|||||||
import crypto from "node:crypto";
|
import crypto from "node:crypto";
|
||||||
import type { OpenClawConfig } from "../../config/config.js";
|
import type { OpenClawConfig } from "../../config/config.js";
|
||||||
import { loadSessionStore, resolveStorePath, type SessionEntry } from "../../config/sessions.js";
|
import {
|
||||||
|
evaluateSessionFreshness,
|
||||||
|
loadSessionStore,
|
||||||
|
resolveSessionResetPolicy,
|
||||||
|
resolveStorePath,
|
||||||
|
type SessionEntry,
|
||||||
|
} from "../../config/sessions.js";
|
||||||
|
|
||||||
export function resolveCronSession(params: {
|
export function resolveCronSession(params: {
|
||||||
cfg: OpenClawConfig;
|
cfg: OpenClawConfig;
|
||||||
sessionKey: string;
|
sessionKey: string;
|
||||||
nowMs: number;
|
nowMs: number;
|
||||||
agentId: string;
|
agentId: string;
|
||||||
|
forceNew?: boolean;
|
||||||
}) {
|
}) {
|
||||||
const sessionCfg = params.cfg.session;
|
const sessionCfg = params.cfg.session;
|
||||||
const storePath = resolveStorePath(sessionCfg?.store, {
|
const storePath = resolveStorePath(sessionCfg?.store, {
|
||||||
@@ -14,12 +21,50 @@ export function resolveCronSession(params: {
|
|||||||
});
|
});
|
||||||
const store = loadSessionStore(storePath);
|
const store = loadSessionStore(storePath);
|
||||||
const entry = store[params.sessionKey];
|
const entry = store[params.sessionKey];
|
||||||
const sessionId = crypto.randomUUID();
|
|
||||||
const systemSent = false;
|
// 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 sessionEntry: SessionEntry = {
|
const sessionEntry: SessionEntry = {
|
||||||
|
// Spread existing entry to preserve conversation context when reusing
|
||||||
|
...(isNewSession ? undefined : entry),
|
||||||
sessionId,
|
sessionId,
|
||||||
updatedAt: params.nowMs,
|
updatedAt: params.nowMs,
|
||||||
systemSent,
|
systemSent,
|
||||||
|
// Preserve user preferences from existing entry
|
||||||
thinkingLevel: entry?.thinkingLevel,
|
thinkingLevel: entry?.thinkingLevel,
|
||||||
verboseLevel: entry?.verboseLevel,
|
verboseLevel: entry?.verboseLevel,
|
||||||
model: entry?.model,
|
model: entry?.model,
|
||||||
@@ -34,5 +79,5 @@ export function resolveCronSession(params: {
|
|||||||
displayName: entry?.displayName,
|
displayName: entry?.displayName,
|
||||||
skillsSnapshot: entry?.skillsSnapshot,
|
skillsSnapshot: entry?.skillsSnapshot,
|
||||||
};
|
};
|
||||||
return { storePath, store, sessionEntry, systemSent, isNewSession: true };
|
return { storePath, store, sessionEntry, systemSent, isNewSession };
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user