diff --git a/CHANGELOG.md b/CHANGELOG.md index 8be3e8b457a..ecbca6c2a41 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,7 +57,7 @@ Docs: https://docs.openclaw.ai - Docs/i18n: relocalize final localized-page links after translation so generated locale pages stop keeping stale English-root links when targets appear later in the same run. (#61796) thanks @hxy91819. - Docs/i18n: remove the zh-CN homepage redirect override so Mintlify can resolve the localized Chinese homepage without self-redirecting `/zh-CN/index`. - Tools/web_fetch and web_search: fix `TypeError: fetch failed` caused by undici 8.0 enabling HTTP/2 by default; pinned SSRF-guard dispatchers now explicitly set `allowH2: false` to restore HTTP/1.1 behavior and keep the custom DNS-pinning lookup compatible. (#61738, #61777) Thanks @zozo123. - +- Agents/session keys: backfill `sessionKey` from `sessionId` in the embedded PI runner when callers omit it, so hooks, LCM, and compaction receive a valid key; also normalize whitespace-only session keys to `undefined` before downstream consumers see them. (#60555) Thanks @100yenadmin. ## 2026.4.5 ### Breaking diff --git a/src/agents/command/session.resolve-session-key.test.ts b/src/agents/command/session.resolve-session-key.test.ts index ad843bc8b6f..0065ef4d62b 100644 --- a/src/agents/command/session.resolve-session-key.test.ts +++ b/src/agents/command/session.resolve-session-key.test.ts @@ -25,7 +25,8 @@ vi.mock("../agent-scope.js", () => ({ listAgentIds: () => hoisted.listAgentIdsMock(), })); -const { resolveSessionKeyForRequest } = await import("./session.js"); +const { resolveSessionKeyForRequest, resolveStoredSessionKeyForSessionId } = + await import("./session.js"); describe("resolveSessionKeyForRequest", () => { beforeEach(() => { @@ -95,4 +96,32 @@ describe("resolveSessionKeyForRequest", () => { expect(result.sessionStore).toBe(otherStore); expect(result.storePath).toBe("/stores/other.json"); }); + + it("scopes stored session-key lookup to the requested agent store", () => { + const embeddedAgentStore = { + "agent:embedded-agent:main": { sessionId: "other-session", updatedAt: 2 }, + "agent:embedded-agent:work": { sessionId: "resume-agent-1", updatedAt: 1 }, + } satisfies Record; + hoisted.loadSessionStoreMock.mockImplementation((storePath) => { + if (storePath === "/stores/embedded-agent.json") { + return embeddedAgentStore; + } + return {}; + }); + + const result = resolveStoredSessionKeyForSessionId({ + cfg: { + session: { + store: "/stores/{agentId}.json", + }, + } satisfies OpenClawConfig, + sessionId: "resume-agent-1", + agentId: "embedded-agent", + }); + + expect(result.sessionKey).toBe("agent:embedded-agent:work"); + expect(result.sessionStore).toBe(embeddedAgentStore); + expect(result.storePath).toBe("/stores/embedded-agent.json"); + expect(hoisted.loadSessionStoreMock).toHaveBeenCalledTimes(1); + }); }); diff --git a/src/agents/command/session.ts b/src/agents/command/session.ts index f46cd53e8bc..0f0c700b573 100644 --- a/src/agents/command/session.ts +++ b/src/agents/command/session.ts @@ -95,6 +95,37 @@ function collectSessionIdMatchesForRequest(opts: { return { matches, primaryStoreMatches, storeByKey }; } +/** + * Resolve an existing stored session key for a session id from a specific agent store. + * This scopes the lookup to the target store without implicitly converting `agentId` + * into that agent's main session key. + */ +export function resolveStoredSessionKeyForSessionId(opts: { + cfg: OpenClawConfig; + sessionId: string; + agentId?: string; +}): SessionKeyResolution { + const sessionId = opts.sessionId.trim(); + const storeAgentId = opts.agentId?.trim() ? normalizeAgentId(opts.agentId) : undefined; + const storePath = resolveStorePath(opts.cfg.session?.store, { + agentId: storeAgentId, + }); + const sessionStore = loadSessionStore(storePath); + if (!sessionId) { + return { sessionKey: undefined, sessionStore, storePath }; + } + + const selection = resolveSessionIdMatchSelection( + Object.entries(sessionStore).filter(([, entry]) => entry?.sessionId === sessionId), + sessionId, + ); + return { + sessionKey: selection.kind === "selected" ? selection.sessionKey : undefined, + sessionStore, + storePath, + }; +} + export function resolveSessionKeyForRequest(opts: { cfg: OpenClawConfig; to?: string; @@ -121,9 +152,10 @@ export function resolveSessionKeyForRequest(opts: { let sessionKey: string | undefined = explicitSessionKey ?? (ctx ? resolveSessionKey(scope, ctx, mainKey) : undefined); - // If a session id was provided, prefer to re-use its entry (by id) even when no key was derived. - // When duplicates exist across agent stores, pick the same deterministic best match used by the - // shared gateway/session resolver helpers instead of whichever store happens to be scanned first. + // If a session id was provided, prefer to re-use its existing entry (by id) even when no key was + // derived. When duplicates exist across agent stores, pick the same deterministic best match used + // by the shared gateway/session resolver helpers instead of whichever store happens to be scanned + // first. if ( opts.sessionId && !explicitSessionKey && diff --git a/src/agents/pi-embedded-runner.e2e.test.ts b/src/agents/pi-embedded-runner.e2e.test.ts index 6f269ec7774..caf23ecdf18 100644 --- a/src/agents/pi-embedded-runner.e2e.test.ts +++ b/src/agents/pi-embedded-runner.e2e.test.ts @@ -18,6 +18,9 @@ const runEmbeddedAttemptMock = vi.fn(); const disposeSessionMcpRuntimeMock = vi.fn<(sessionId: string) => Promise>(async () => { return undefined; }); +const resolveSessionKeyForRequestMock = vi.fn(); +const resolveStoredSessionKeyForSessionIdMock = vi.fn(); +const loggerWarnMock = vi.fn(); let refreshRuntimeAuthOnFirstPromptError = false; vi.mock("@mariozechner/pi-ai", async () => { @@ -95,6 +98,28 @@ const installRunEmbeddedMocks = () => { vi.doMock("./runtime-plugins.js", () => ({ ensureRuntimePluginsLoaded: vi.fn(), })); + vi.doMock("./command/session.js", async () => { + const actual = + await vi.importActual("./command/session.js"); + return { + ...actual, + resolveSessionKeyForRequest: (opts: unknown) => resolveSessionKeyForRequestMock(opts), + resolveStoredSessionKeyForSessionId: (opts: unknown) => + resolveStoredSessionKeyForSessionIdMock(opts), + }; + }); + vi.doMock("./pi-embedded-runner/logger.js", async () => { + const actual = await vi.importActual( + "./pi-embedded-runner/logger.js", + ); + return { + ...actual, + log: { + ...actual.log, + warn: (...args: unknown[]) => loggerWarnMock(...args), + }, + }; + }); vi.doMock("./pi-embedded-runner/run/attempt.js", () => ({ runEmbeddedAttempt: (params: unknown) => runEmbeddedAttemptMock(params), })); @@ -166,6 +191,9 @@ beforeEach(() => { vi.useRealTimers(); runEmbeddedAttemptMock.mockReset(); disposeSessionMcpRuntimeMock.mockReset(); + resolveSessionKeyForRequestMock.mockReset(); + resolveStoredSessionKeyForSessionIdMock.mockReset(); + loggerWarnMock.mockReset(); refreshRuntimeAuthOnFirstPromptError = false; runEmbeddedAttemptMock.mockImplementation(async () => { throw new Error("unexpected extra runEmbeddedAttempt call"); @@ -268,6 +296,165 @@ const runDefaultEmbeddedTurn = async (sessionFile: string, prompt: string, sessi }; describe("runEmbeddedPiAgent", () => { + it("backfills a trimmed session key from sessionId when the embedded run omits it", async () => { + const sessionFile = nextSessionFile(); + const cfg = createEmbeddedPiRunnerOpenAiConfig(["mock-1"]); + resolveSessionKeyForRequestMock.mockReturnValue({ + sessionKey: "agent:test:resolved", + sessionStore: {}, + storePath: "/tmp/session-store.json", + }); + runEmbeddedAttemptMock.mockResolvedValueOnce( + makeEmbeddedRunnerAttempt({ + assistantTexts: ["ok"], + lastAssistant: buildEmbeddedRunnerAssistant({ + content: [{ type: "text", text: "ok" }], + }), + }), + ); + + await runEmbeddedPiAgent({ + sessionId: "resume-123", + sessionKey: " ", + sessionFile, + workspaceDir, + config: cfg, + prompt: "hello", + provider: "openai", + model: "mock-1", + timeoutMs: 5_000, + agentDir, + runId: nextRunId("backfill"), + enqueue: immediateEnqueue, + }); + + expect(resolveSessionKeyForRequestMock).toHaveBeenCalledWith({ + cfg, + sessionId: "resume-123", + agentId: undefined, + }); + const firstCall = runEmbeddedAttemptMock.mock.calls[0]?.[0] as { sessionKey?: string }; + expect(firstCall.sessionKey).toBe("agent:test:resolved"); + }); + + it("drops whitespace-only session keys when backfill cannot resolve a session key", async () => { + const sessionFile = nextSessionFile(); + const cfg = createEmbeddedPiRunnerOpenAiConfig(["mock-1"]); + resolveSessionKeyForRequestMock.mockReturnValue({ + sessionKey: undefined, + sessionStore: {}, + storePath: "/tmp/session-store.json", + }); + runEmbeddedAttemptMock.mockResolvedValueOnce( + makeEmbeddedRunnerAttempt({ + assistantTexts: ["ok"], + lastAssistant: buildEmbeddedRunnerAssistant({ + content: [{ type: "text", text: "ok" }], + }), + }), + ); + + await runEmbeddedPiAgent({ + sessionId: "resume-124", + sessionKey: " ", + sessionFile, + workspaceDir, + config: cfg, + prompt: "hello", + provider: "openai", + model: "mock-1", + timeoutMs: 5_000, + agentDir, + runId: nextRunId("backfill-empty"), + enqueue: immediateEnqueue, + }); + + expect(resolveSessionKeyForRequestMock).toHaveBeenCalledWith({ + cfg, + sessionId: "resume-124", + agentId: undefined, + }); + const firstCall = runEmbeddedAttemptMock.mock.calls[0]?.[0] as { sessionKey?: string }; + expect(firstCall.sessionKey).toBeUndefined(); + }); + + it("logs when embedded session-key backfill resolution fails", async () => { + const sessionFile = nextSessionFile(); + const cfg = createEmbeddedPiRunnerOpenAiConfig(["mock-1"]); + resolveSessionKeyForRequestMock.mockImplementation(() => { + throw new Error("resolver exploded"); + }); + runEmbeddedAttemptMock.mockResolvedValueOnce( + makeEmbeddedRunnerAttempt({ + assistantTexts: ["ok"], + lastAssistant: buildEmbeddedRunnerAssistant({ + content: [{ type: "text", text: "ok" }], + }), + }), + ); + + await runEmbeddedPiAgent({ + sessionId: "resume-456", + sessionFile, + workspaceDir, + config: cfg, + prompt: "hello", + provider: "openai", + model: "mock-1", + timeoutMs: 5_000, + agentDir, + runId: nextRunId("backfill-warn"), + enqueue: immediateEnqueue, + }); + + expect( + loggerWarnMock.mock.calls.some(([message]) => + String(message ?? "").includes("[backfillSessionKey] Failed to resolve sessionKey"), + ), + ).toBe(true); + }); + + it("passes the current agentId when backfilling a session key", async () => { + const sessionFile = nextSessionFile(); + const cfg = createEmbeddedPiRunnerOpenAiConfig(["mock-1"]); + resolveStoredSessionKeyForSessionIdMock.mockReturnValue({ + sessionKey: "agent:test:resolved", + sessionStore: {}, + storePath: "/tmp/session-store.json", + }); + runEmbeddedAttemptMock.mockResolvedValueOnce( + makeEmbeddedRunnerAttempt({ + assistantTexts: ["ok"], + lastAssistant: buildEmbeddedRunnerAssistant({ + content: [{ type: "text", text: "ok" }], + }), + }), + ); + + await runEmbeddedPiAgent({ + sessionId: "resume-agent-1", + sessionKey: undefined, + sessionFile, + workspaceDir, + config: cfg, + prompt: "hello", + provider: "openai", + model: "mock-1", + timeoutMs: 5_000, + agentDir, + agentId: "embedded-agent", + runId: nextRunId("backfill-agent-scope"), + enqueue: immediateEnqueue, + }); + + expect(resolveStoredSessionKeyForSessionIdMock).toHaveBeenCalledWith({ + cfg, + sessionId: "resume-agent-1", + agentId: "embedded-agent", + }); + expect(resolveSessionKeyForRequestMock).not.toHaveBeenCalled(); + }); + it("disposes bundle MCP once when a one-shot local run completes", async () => { const sessionFile = nextSessionFile(); const cfg = createEmbeddedPiRunnerOpenAiConfig(["mock-1"]); @@ -353,7 +540,7 @@ describe("runEmbeddedPiAgent", () => { runEmbeddedAttemptMock .mockImplementationOnce(async (params: unknown) => { - expect((params as { prompt?: string }).prompt).toBe("ship it"); + expect((params as { prompt?: string }).prompt).toMatch(/^ship it(?:\n\n|$)/); return makeEmbeddedRunnerAttempt({ assistantTexts: ["I'll inspect the files, make the change, and run the checks."], lastAssistant: buildEmbeddedRunnerAssistant({ diff --git a/src/agents/pi-embedded-runner/run.ts b/src/agents/pi-embedded-runner/run.ts index d4dc99b4692..b60cb1a0082 100644 --- a/src/agents/pi-embedded-runner/run.ts +++ b/src/agents/pi-embedded-runner/run.ts @@ -19,6 +19,10 @@ import { markAuthProfileGood, markAuthProfileUsed, } from "../auth-profiles.js"; +import { + resolveSessionKeyForRequest, + resolveStoredSessionKeyForSessionId, +} from "../command/session.js"; import { DEFAULT_MODEL, DEFAULT_PROVIDER } from "../defaults.js"; import { coerceToFailoverError, @@ -101,9 +105,60 @@ import { describeUnknownError } from "./utils.js"; type ApiKeyInfo = ResolvedProviderAuth; +/** + * Best-effort backfill of sessionKey from sessionId when not explicitly provided. + * The return value is normalized: whitespace-only inputs collapse to undefined, and + * successful resolution returns a trimmed session key. This is a read-only lookup + * with no side effects. + * See: https://github.com/openclaw/openclaw/issues/60552 + */ +function backfillSessionKey(params: { + config: RunEmbeddedPiAgentParams["config"]; + sessionId: string; + sessionKey?: string; + agentId?: string; +}): string | undefined { + const trimmed = params.sessionKey?.trim() || undefined; + if (trimmed) { + return trimmed; + } + if (!params.config || !params.sessionId) { + return undefined; + } + try { + const resolved = params.agentId?.trim() + ? resolveStoredSessionKeyForSessionId({ + cfg: params.config, + sessionId: params.sessionId, + agentId: params.agentId, + }) + : resolveSessionKeyForRequest({ + cfg: params.config, + sessionId: params.sessionId, + }); + return resolved.sessionKey?.trim() || undefined; + } catch (err) { + log.warn( + `[backfillSessionKey] Failed to resolve sessionKey for sessionId=${redactRunIdentifier(sanitizeForLog(params.sessionId))}: ${describeUnknownError(err)}`, + ); + return undefined; + } +} + export async function runEmbeddedPiAgent( params: RunEmbeddedPiAgentParams, ): Promise { + // Resolve sessionKey early so all downstream consumers (hooks, LCM, compaction) + // receive a non-null key even when callers omit it. See #60552. + const effectiveSessionKey = backfillSessionKey({ + config: params.config, + sessionId: params.sessionId, + sessionKey: params.sessionKey, + agentId: params.agentId, + }); + if (effectiveSessionKey !== params.sessionKey) { + params = { ...params, sessionKey: effectiveSessionKey }; + } const sessionLane = resolveSessionLane(params.sessionKey?.trim() || params.sessionId); const globalLane = resolveGlobalLane(params.lane); const enqueueGlobal = @@ -167,17 +222,19 @@ export async function runEmbeddedPiAgent( let provider = (params.provider ?? DEFAULT_PROVIDER).trim() || DEFAULT_PROVIDER; let modelId = (params.model ?? DEFAULT_MODEL).trim() || DEFAULT_MODEL; const agentDir = params.agentDir ?? resolveOpenClawAgentDir(); + const normalizedSessionKey = params.sessionKey?.trim(); const fallbackConfigured = hasConfiguredModelFallbacks({ cfg: params.config, agentId: params.agentId, - sessionKey: params.sessionKey, + sessionKey: normalizedSessionKey, }); await ensureOpenClawModelsJson(params.config, agentDir); + const resolvedSessionKey = normalizedSessionKey; const hookRunner = getGlobalHookRunner(); const hookCtx = { runId: params.runId, agentId: workspaceResolution.agentId, - sessionKey: params.sessionKey, + sessionKey: resolvedSessionKey, sessionId: params.sessionId, workspaceDir: resolvedWorkspace, modelProviderId: provider, @@ -526,7 +583,7 @@ export async function runEmbeddedPiAgent( const attempt = await runEmbeddedAttempt({ sessionId: params.sessionId, - sessionKey: params.sessionKey, + sessionKey: resolvedSessionKey, trigger: params.trigger, memoryFlushWritePath: params.memoryFlushWritePath, messageChannel: params.messageChannel, @@ -649,7 +706,7 @@ export async function runEmbeddedPiAgent( const formattedAssistantErrorText = lastAssistant ? formatAssistantErrorText(lastAssistant, { cfg: params.config, - sessionKey: params.sessionKey ?? params.sessionId, + sessionKey: resolvedSessionKey ?? params.sessionId, provider: activeErrorContext.provider, model: activeErrorContext.model, }) @@ -673,7 +730,7 @@ export async function runEmbeddedPiAgent( } const requestedSelection = shouldSwitchToLiveModel({ cfg: params.config, - sessionKey: params.sessionKey, + sessionKey: resolvedSessionKey, agentId: params.agentId, defaultProvider: DEFAULT_PROVIDER, defaultModel: DEFAULT_MODEL, @@ -685,7 +742,7 @@ export async function runEmbeddedPiAgent( if (requestedSelection && canRestartForLiveSwitch) { await clearLiveModelSwitchPending({ cfg: params.config, - sessionKey: params.sessionKey, + sessionKey: resolvedSessionKey, agentId: params.agentId, }); log.info( diff --git a/src/gateway/sessions-history-http.test.ts b/src/gateway/sessions-history-http.test.ts index 33c2b984adc..caa92aed192 100644 --- a/src/gateway/sessions-history-http.test.ts +++ b/src/gateway/sessions-history-http.test.ts @@ -368,6 +368,13 @@ describe("session history HTTP endpoints", () => { .messages?.[0]?.content?.[0]?.text, ).toBe("second message"); + const suppressed = await appendAssistantMessageToSessionTranscript({ + sessionKey: "agent:main:main", + text: "NO_REPLY", + storePath, + }); + expect(suppressed.ok).toBe(true); + const appended = await appendAssistantMessageToSessionTranscript({ sessionKey: "agent:main:main", text: "third message", @@ -375,12 +382,21 @@ describe("session history HTTP endpoints", () => { }); expect(appended.ok).toBe(true); + const suppressedEvent = await readSseEvent(reader!, streamState); + expect(suppressedEvent.event).toBe("history"); + const suppressedData = suppressedEvent.data as { + messages?: Array<{ content?: Array<{ text?: string }>; __openclaw?: { seq?: number } }>; + }; + expect(suppressedData.messages?.[0]?.content?.[0]?.text).toBe("NO_REPLY"); + expect(suppressedData.messages?.[0]?.__openclaw?.seq).toBe(3); + const nextEvent = await readSseEvent(reader!, streamState); expect(nextEvent.event).toBe("history"); - expect( - (nextEvent.data as { messages?: Array<{ content?: Array<{ text?: string }> }> }) - .messages?.[0]?.content?.[0]?.text, - ).toBe("third message"); + const nextData = nextEvent.data as { + messages?: Array<{ content?: Array<{ text?: string }>; __openclaw?: { seq?: number } }>; + }; + expect(nextData.messages?.[0]?.content?.[0]?.text).toBe("third message"); + expect(nextData.messages?.[0]?.__openclaw?.seq).toBe(4); await reader?.cancel(); });