From f40f8a60cce5ea506c992fb83d84f8db82c640c3 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Thu, 23 Apr 2026 12:32:30 -0700 Subject: [PATCH] fix(agents): harden cli runner hook followups (#70747) * fix(agents): harden cli runner hook followups * fix(agents): harden cli runner hook followups * fix(agents): harden cli runner hook followups * fix(agents): harden cli runner hook followups * fix(agents): harden cli runner hook followups * fix(agents): harden cli runner hook followups --- src/agents/cli-runner.reliability.test.ts | 171 ++++++++++++++++-- src/agents/cli-runner.ts | 141 ++++++++------- src/agents/cli-runner/prepare.test.ts | 90 +++++---- src/agents/cli-runner/prepare.ts | 15 +- src/agents/cli-runner/session-history.test.ts | 152 ++++++++++++++++ src/agents/cli-runner/session-history.ts | 67 +++++++ 6 files changed, 510 insertions(+), 126 deletions(-) create mode 100644 src/agents/cli-runner/session-history.test.ts create mode 100644 src/agents/cli-runner/session-history.ts diff --git a/src/agents/cli-runner.reliability.test.ts b/src/agents/cli-runner.reliability.test.ts index 1aa5c218705..0e5bea8a454 100644 --- a/src/agents/cli-runner.reliability.test.ts +++ b/src/agents/cli-runner.reliability.test.ts @@ -1,7 +1,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { CURRENT_SESSION_VERSION, SessionManager } from "@mariozechner/pi-coding-agent"; +import { CURRENT_SESSION_VERSION } from "@mariozechner/pi-coding-agent"; import { afterEach, describe, expect, it, vi } from "vitest"; import { __testing as replyRunTesting, @@ -18,6 +18,7 @@ import { } from "./cli-runner.test-support.js"; import { executePreparedCliRun } from "./cli-runner/execute.js"; import { resolveCliNoOutputTimeoutMs } from "./cli-runner/helpers.js"; +import { MAX_CLI_SESSION_HISTORY_MESSAGES } from "./cli-runner/session-history.js"; import type { PreparedCliRunContext } from "./cli-runner/types.js"; vi.mock("../plugins/hook-runner-global.js", async () => { @@ -34,7 +35,9 @@ const mockGetGlobalHookRunner = vi.mocked(getGlobalHookRunner); function createSessionFile(params?: { history?: Array<{ role: "user"; content: string }> }) { const dir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-cli-hooks-")); - const sessionFile = path.join(dir, "session.jsonl"); + vi.stubEnv("OPENCLAW_STATE_DIR", dir); + const sessionFile = path.join(dir, "agents", "main", "sessions", "s1.jsonl"); + fs.mkdirSync(path.dirname(sessionFile), { recursive: true }); fs.writeFileSync( sessionFile, `${JSON.stringify({ @@ -46,13 +49,22 @@ function createSessionFile(params?: { history?: Array<{ role: "user"; content: s })}\n`, "utf-8", ); - const sessionManager = SessionManager.open(sessionFile); - for (const entry of params?.history ?? []) { - sessionManager.appendMessage({ - role: entry.role, - content: entry.content, - timestamp: Date.now(), - }); + for (const [index, entry] of (params?.history ?? []).entries()) { + fs.appendFileSync( + sessionFile, + `${JSON.stringify({ + type: "message", + id: `msg-${index}`, + parentId: index > 0 ? `msg-${index - 1}` : null, + timestamp: new Date(index + 1).toISOString(), + message: { + role: entry.role, + content: entry.content, + timestamp: index + 1, + }, + })}\n`, + "utf-8", + ); } return { dir, sessionFile }; } @@ -110,6 +122,7 @@ describe("runCliAgent reliability", () => { afterEach(() => { replyRunTesting.resetReplyRunRegistry(); mockGetGlobalHookRunner.mockReset(); + vi.unstubAllEnvs(); }); it("fails with timeout when no-output watchdog trips", async () => { @@ -193,6 +206,13 @@ describe("runCliAgent reliability", () => { }); it("rethrows the retry failure when session-expired recovery retry also fails", async () => { + const hookRunner = { + hasHooks: vi.fn((hookName: string) => ["llm_input", "agent_end"].includes(hookName)), + runLlmInput: vi.fn(async () => undefined), + runLlmOutput: vi.fn(async () => undefined), + runAgentEnd: vi.fn(async () => undefined), + }; + mockGetGlobalHookRunner.mockReturnValue(hookRunner as never); supervisorSpawnMock.mockClear(); supervisorSpawnMock.mockResolvedValueOnce( createManagedRun({ @@ -218,18 +238,50 @@ describe("runCliAgent reliability", () => { noOutputTimedOut: false, }), ); + const { dir, sessionFile } = createSessionFile({ + history: [{ role: "user", content: "earlier context" }], + }); - await expect( - runPreparedCliAgent( - buildPreparedContext({ - sessionKey: "agent:main:subagent:retry", - runId: "run-retry-failure", - cliSessionId: "thread-123", + try { + await expect( + runPreparedCliAgent({ + ...buildPreparedContext({ + sessionKey: "agent:main:subagent:retry", + runId: "run-retry-failure", + cliSessionId: "thread-123", + }), + params: { + ...buildPreparedContext({ + sessionKey: "agent:main:subagent:retry", + runId: "run-retry-failure", + cliSessionId: "thread-123", + }).params, + agentId: "main", + sessionFile, + workspaceDir: dir, + }, }), - ), - ).rejects.toThrow("rate limit exceeded"); + ).rejects.toThrow("rate limit exceeded"); - expect(supervisorSpawnMock).toHaveBeenCalledTimes(2); + expect(supervisorSpawnMock).toHaveBeenCalledTimes(2); + await vi.waitFor(() => { + expect(hookRunner.runLlmInput).toHaveBeenCalledTimes(1); + expect(hookRunner.runAgentEnd).toHaveBeenCalledTimes(1); + }); + expect(hookRunner.runAgentEnd).toHaveBeenCalledWith( + expect.objectContaining({ + success: false, + error: "rate limit exceeded", + messages: [ + { role: "user", content: "earlier context", timestamp: expect.any(Number) }, + { role: "user", content: "hi", timestamp: expect.any(Number) }, + ], + }), + expect.any(Object), + ); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } }); it("returns the assembled CLI prompt in meta for raw trace consumers", async () => { @@ -499,6 +551,89 @@ describe("runCliAgent reliability", () => { expect.any(Object), ); }); + + it("does not emit duplicate llm_input when session-expired recovery succeeds", async () => { + const hookRunner = { + hasHooks: vi.fn((hookName: string) => + ["llm_input", "llm_output", "agent_end"].includes(hookName), + ), + runLlmInput: vi.fn(async () => undefined), + runLlmOutput: vi.fn(async () => undefined), + runAgentEnd: vi.fn(async () => undefined), + }; + mockGetGlobalHookRunner.mockReturnValue(hookRunner as never); + const { dir, sessionFile } = createSessionFile({ + history: Array.from({ length: MAX_CLI_SESSION_HISTORY_MESSAGES + 5 }, (_, index) => ({ + role: "user" as const, + content: `history-${index}`, + })), + }); + + supervisorSpawnMock.mockResolvedValueOnce( + createManagedRun({ + reason: "exit", + exitCode: 1, + exitSignal: null, + durationMs: 50, + stdout: "", + stderr: "session expired", + timedOut: false, + noOutputTimedOut: false, + }), + ); + supervisorSpawnMock.mockResolvedValueOnce( + createManagedRun({ + reason: "exit", + exitCode: 0, + exitSignal: null, + durationMs: 50, + stdout: "recovered output", + stderr: "", + timedOut: false, + noOutputTimedOut: false, + }), + ); + + try { + await expect( + runPreparedCliAgent({ + ...buildPreparedContext({ + sessionKey: "agent:main:main", + runId: "run-retry-success", + cliSessionId: "thread-123", + }), + params: { + ...buildPreparedContext({ + sessionKey: "agent:main:main", + runId: "run-retry-success", + cliSessionId: "thread-123", + }).params, + agentId: "main", + sessionFile, + workspaceDir: dir, + }, + }), + ).resolves.toMatchObject({ + payloads: [{ text: "recovered output" }], + }); + + await vi.waitFor(() => { + expect(hookRunner.runLlmInput).toHaveBeenCalledTimes(1); + expect(hookRunner.runLlmOutput).toHaveBeenCalledTimes(1); + expect(hookRunner.runAgentEnd).toHaveBeenCalledTimes(1); + }); + const llmInputCalls = hookRunner.runLlmInput.mock.calls as unknown as Array>; + const llmInputEvent = llmInputCalls[0]?.[0] as { historyMessages: unknown[] } | undefined; + expect(llmInputEvent).toBeDefined(); + expect(llmInputEvent?.historyMessages).toHaveLength(MAX_CLI_SESSION_HISTORY_MESSAGES); + expect(llmInputEvent?.historyMessages[0]).toMatchObject({ + role: "user", + content: `history-5`, + }); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); }); describe("resolveCliNoOutputTimeoutMs", () => { diff --git a/src/agents/cli-runner.ts b/src/agents/cli-runner.ts index 961fac5319b..33771ed65c4 100644 --- a/src/agents/cli-runner.ts +++ b/src/agents/cli-runner.ts @@ -1,5 +1,5 @@ -import { SessionManager } from "@mariozechner/pi-coding-agent"; import { formatErrorMessage } from "../infra/errors.js"; +import { loadCliSessionHistoryMessages } from "./cli-runner/session-history.js"; import type { PreparedCliRunContext, RunCliAgentParams } from "./cli-runner/types.js"; import { FailoverError, isFailoverError, resolveFailoverStatus } from "./failover-error.js"; import { @@ -10,15 +10,6 @@ import { import { classifyFailoverReason, isFailoverErrorMessage } from "./pi-embedded-helpers.js"; import type { EmbeddedPiRunResult } from "./pi-embedded-runner.js"; -function loadCliHookHistoryMessages(sessionFile: string): unknown[] { - try { - const entries = SessionManager.open(sessionFile).getEntries(); - return entries.flatMap((entry) => (entry.type === "message" ? [entry.message as unknown] : [])); - } catch { - return []; - } -} - function buildCliHookUserMessage(prompt: string): unknown { return { role: "user", @@ -62,7 +53,13 @@ export async function runPreparedCliAgent( ): Promise { const { executePreparedCliRun } = await import("./cli-runner/execute.runtime.js"); const { params } = context; - const historyMessages = loadCliHookHistoryMessages(params.sessionFile); + const historyMessages = loadCliSessionHistoryMessages({ + sessionId: params.sessionId, + sessionFile: params.sessionFile, + sessionKey: params.sessionKey, + agentId: params.agentId, + config: params.config, + }); const llmInputEvent = { runId: params.runId, sessionId: params.sessionId, @@ -84,11 +81,38 @@ export async function runPreparedCliAgent( channelId: params.messageChannel ?? params.messageProvider, } as const; - const runCliAttempt = async (cliSessionIdToUse?: string) => { - runAgentHarnessLlmInputHook({ - event: llmInputEvent, - ctx: hookContext, - }); + const buildAgentEndMessages = (lastAssistant?: unknown): unknown[] => [ + ...historyMessages, + buildCliHookUserMessage(params.prompt), + ...(lastAssistant ? [lastAssistant] : []), + ]; + + const buildFailedAgentEndEvent = (error: string) => ({ + messages: buildAgentEndMessages(), + success: false, + error, + durationMs: Date.now() - context.started, + }); + + const toCliRunFailure = (error: unknown): never => { + if (isFailoverError(error)) { + throw error; + } + const message = formatErrorMessage(error); + if (isFailoverErrorMessage(message, { provider: params.provider })) { + const reason = classifyFailoverReason(message, { provider: params.provider }) ?? "unknown"; + const status = resolveFailoverStatus(reason); + throw new FailoverError(message, { + reason, + provider: params.provider, + model: context.modelId, + status, + }); + } + throw error; + }; + + const executeCliAttempt = async (cliSessionIdToUse?: string) => { const output = await executePreparedCliRun(context, cliSessionIdToUse); const assistantText = output.text.trim(); const assistantTexts = assistantText ? [assistantText] : []; @@ -193,16 +217,18 @@ export async function runPreparedCliAgent( // Try with the provided CLI session ID first try { + runAgentHarnessLlmInputHook({ + event: llmInputEvent, + ctx: hookContext, + }); try { - const { output, lastAssistant } = await runCliAttempt(context.reusableCliSession.sessionId); + const { output, lastAssistant } = await executeCliAttempt( + context.reusableCliSession.sessionId, + ); const effectiveCliSessionId = output.sessionId ?? context.reusableCliSession.sessionId; runAgentHarnessAgentEndHook({ event: { - messages: [ - ...historyMessages, - buildCliHookUserMessage(params.prompt), - ...(lastAssistant ? [lastAssistant] : []), - ], + messages: buildAgentEndMessages(lastAssistant), success: true, durationMs: Date.now() - context.started, }, @@ -219,64 +245,39 @@ export async function runPreparedCliAgent( // We'll need to modify the caller to handle this case // For now, retry without the session ID to create a new session - const { output, lastAssistant } = await runCliAttempt(undefined); - const effectiveCliSessionId = output.sessionId; - runAgentHarnessAgentEndHook({ - event: { - messages: [ - ...historyMessages, - buildCliHookUserMessage(params.prompt), - ...(lastAssistant ? [lastAssistant] : []), - ], - success: true, - durationMs: Date.now() - context.started, - }, - ctx: hookContext, - }); - return buildCliRunResult({ output, effectiveCliSessionId }); + try { + const { output, lastAssistant } = await executeCliAttempt(undefined); + const effectiveCliSessionId = output.sessionId; + runAgentHarnessAgentEndHook({ + event: { + messages: buildAgentEndMessages(lastAssistant), + success: true, + durationMs: Date.now() - context.started, + }, + ctx: hookContext, + }); + return buildCliRunResult({ output, effectiveCliSessionId }); + } catch (retryErr) { + const retryMessage = formatErrorMessage(retryErr); + runAgentHarnessAgentEndHook({ + event: buildFailedAgentEndEvent(retryMessage), + ctx: hookContext, + }); + return toCliRunFailure(retryErr); + } } runAgentHarnessAgentEndHook({ - event: { - messages: [...historyMessages, buildCliHookUserMessage(params.prompt)], - success: false, - error: formatErrorMessage(err), - durationMs: Date.now() - context.started, - }, + event: buildFailedAgentEndEvent(formatErrorMessage(err)), ctx: hookContext, }); throw err; } const message = formatErrorMessage(err); - if (isFailoverErrorMessage(message, { provider: params.provider })) { - const reason = classifyFailoverReason(message, { provider: params.provider }) ?? "unknown"; - const status = resolveFailoverStatus(reason); - const failoverError = new FailoverError(message, { - reason, - provider: params.provider, - model: context.modelId, - status, - }); - runAgentHarnessAgentEndHook({ - event: { - messages: [...historyMessages, buildCliHookUserMessage(params.prompt)], - success: false, - error: message, - durationMs: Date.now() - context.started, - }, - ctx: hookContext, - }); - throw failoverError; - } runAgentHarnessAgentEndHook({ - event: { - messages: [...historyMessages, buildCliHookUserMessage(params.prompt)], - success: false, - error: message, - durationMs: Date.now() - context.started, - }, + event: buildFailedAgentEndEvent(message), ctx: hookContext, }); - throw err; + return toCliRunFailure(err); } } finally { await context.preparedBackend.cleanup?.(); diff --git a/src/agents/cli-runner/prepare.test.ts b/src/agents/cli-runner/prepare.test.ts index a0f758e5859..ee96e9e328d 100644 --- a/src/agents/cli-runner/prepare.test.ts +++ b/src/agents/cli-runner/prepare.test.ts @@ -1,11 +1,10 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { CURRENT_SESSION_VERSION, SessionManager } from "@mariozechner/pi-coding-agent"; +import { CURRENT_SESSION_VERSION } from "@mariozechner/pi-coding-agent"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; import { getGlobalHookRunner } from "../../plugins/hook-runner-global.js"; -import { __testing as cliBackendsTesting } from "../cli-backends.js"; import { buildActiveMusicGenerationTaskPromptContextForSession } from "../music-generation-task-status.js"; import { buildActiveVideoGenerationTaskPromptContextForSession } from "../video-generation-task-status.js"; import { @@ -52,11 +51,10 @@ const mockBuildActiveMusicGenerationTaskPromptContextForSession = vi.mocked( buildActiveMusicGenerationTaskPromptContextForSession, ); -function createCliBackendConfig(opts: { systemPromptOverride?: string } = {}): OpenClawConfig { +function createCliBackendConfig(): OpenClawConfig { return { agents: { defaults: { - ...(opts.systemPromptOverride ? { systemPromptOverride: opts.systemPromptOverride } : {}), cliBackends: { "test-cli": { command: "test-cli", @@ -75,7 +73,9 @@ function createCliBackendConfig(opts: { systemPromptOverride?: string } = {}): O function createSessionFile() { const dir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-cli-prepare-")); - const sessionFile = path.join(dir, "session.jsonl"); + vi.stubEnv("OPENCLAW_STATE_DIR", dir); + const sessionFile = path.join(dir, "agents", "main", "sessions", "session-test.jsonl"); + fs.mkdirSync(path.dirname(sessionFile), { recursive: true }); fs.writeFileSync( sessionFile, `${JSON.stringify({ @@ -90,6 +90,28 @@ function createSessionFile() { return { dir, sessionFile }; } +function appendTranscriptEntry( + sessionFile: string, + entry: { + id: string; + parentId: string | null; + timestamp: string; + message: unknown; + }, +): void { + fs.appendFileSync( + sessionFile, + `${JSON.stringify({ + type: "message", + id: entry.id, + parentId: entry.parentId, + timestamp: entry.timestamp, + message: entry.message, + })}\n`, + "utf-8", + ); +} + describe("shouldSkipLocalCliCredentialEpoch", () => { beforeEach(() => { setCliRunnerPrepareTestDeps({ @@ -100,20 +122,16 @@ describe("shouldSkipLocalCliCredentialEpoch", () => { })), resolveOpenClawDocsPath: vi.fn(async () => null), }); - cliBackendsTesting.setDepsForTest({ - resolvePluginSetupCliBackend: vi.fn(() => undefined), - resolveRuntimeCliBackends: vi.fn(() => []), - }); mockGetGlobalHookRunner.mockReturnValue(null); mockBuildActiveVideoGenerationTaskPromptContextForSession.mockReturnValue(undefined); mockBuildActiveMusicGenerationTaskPromptContextForSession.mockReturnValue(undefined); }); afterEach(() => { - cliBackendsTesting.resetDepsForTest(); mockGetGlobalHookRunner.mockReset(); mockBuildActiveVideoGenerationTaskPromptContextForSession.mockReset(); mockBuildActiveMusicGenerationTaskPromptContextForSession.mockReset(); + vi.unstubAllEnvs(); }); it("skips local cli auth only when a profile-owned execution was prepared", () => { @@ -151,24 +169,33 @@ describe("shouldSkipLocalCliCredentialEpoch", () => { it("applies prompt-build hook context to Claude-style CLI preparation", async () => { const { dir, sessionFile } = createSessionFile(); try { - const sessionManager = SessionManager.open(sessionFile); - sessionManager.appendMessage({ role: "user", content: "earlier context", timestamp: 1 }); - sessionManager.appendMessage({ - role: "assistant", - content: [{ type: "text", text: "earlier reply" }], - api: "responses", - provider: "test-cli", - model: "test-model", - usage: { - input: 0, - output: 0, - cacheRead: 0, - cacheWrite: 0, - totalTokens: 0, - cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + appendTranscriptEntry(sessionFile, { + id: "msg-1", + parentId: null, + timestamp: new Date(1).toISOString(), + message: { role: "user", content: "earlier context", timestamp: 1 }, + }); + appendTranscriptEntry(sessionFile, { + id: "msg-2", + parentId: "msg-1", + timestamp: new Date(2).toISOString(), + message: { + role: "assistant", + content: [{ type: "text", text: "earlier reply" }], + api: "responses", + provider: "test-cli", + model: "test-model", + usage: { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 0, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + stopReason: "stop", + timestamp: 2, }, - stopReason: "stop", - timestamp: 2, }); const hookRunner = { hasHooks: vi.fn((hookName: string) => hookName === "before_prompt_build"), @@ -197,7 +224,7 @@ describe("shouldSkipLocalCliCredentialEpoch", () => { messageChannel: "telegram", messageProvider: "acp", config: { - ...createCliBackendConfig({ systemPromptOverride: "base system" }), + ...createCliBackendConfig(), }, }); @@ -274,7 +301,7 @@ describe("shouldSkipLocalCliCredentialEpoch", () => { model: "test-model", timeoutMs: 1_000, runId: "run-test-legacy-merge", - config: createCliBackendConfig({ systemPromptOverride: "base system" }), + config: createCliBackendConfig(), }); expect(context.params.prompt).toBe("prompt prepend\n\nlegacy prepend\n\nlatest ask"); @@ -309,7 +336,8 @@ describe("shouldSkipLocalCliCredentialEpoch", () => { model: "test-model", timeoutMs: 1_000, runId: "run-test-hook-failure", - config: createCliBackendConfig({ systemPromptOverride: "base extra system" }), + extraSystemPrompt: "base extra system", + config: createCliBackendConfig(), }); expect(context.params.prompt).toBe("latest ask"); @@ -348,7 +376,7 @@ describe("shouldSkipLocalCliCredentialEpoch", () => { model: "test-model", timeoutMs: 1_000, runId: "run-test-prepend-helper", - config: createCliBackendConfig({ systemPromptOverride: "base system" }), + config: createCliBackendConfig(), }); expect(context.systemPrompt).toBe("active video task\n\nhook prepend system\n\nhook system"); diff --git a/src/agents/cli-runner/prepare.ts b/src/agents/cli-runner/prepare.ts index 900d27982e4..a398bad9989 100644 --- a/src/agents/cli-runner/prepare.ts +++ b/src/agents/cli-runner/prepare.ts @@ -1,4 +1,3 @@ -import { SessionManager } from "@mariozechner/pi-coding-agent"; import { ensureMcpLoopbackServer } from "../../gateway/mcp-http.js"; import { createMcpLoopbackServerConfig, @@ -43,6 +42,7 @@ import { redactRunIdentifier, resolveRunWorkspaceDir } from "../workspace-run.js import { prepareCliBundleMcpConfig } from "./bundle-mcp.js"; import { buildSystemPrompt, normalizeCliModel } from "./helpers.js"; import { cliBackendLog } from "./log.js"; +import { loadCliSessionHistoryMessages } from "./session-history.js"; import type { PreparedCliRunContext, RunCliAgentParams } from "./types.js"; const prepareDeps = { @@ -56,11 +56,6 @@ const prepareDeps = { ) => (await import("../docs-path.js")).resolveOpenClawDocsPath(params), }; -function loadCliPromptBuildMessages(sessionFile: string): unknown[] { - const entries = SessionManager.open(sessionFile).getEntries(); - return entries.flatMap((entry) => (entry.type === "message" ? [entry.message as unknown] : [])); -} - export function setCliRunnerPrepareTestDeps(overrides: Partial): void { Object.assign(prepareDeps, overrides); } @@ -315,7 +310,13 @@ export async function prepareCliRunContext( try { const hookResult = await resolvePromptBuildHookResult({ prompt: params.prompt, - messages: loadCliPromptBuildMessages(params.sessionFile), + messages: loadCliSessionHistoryMessages({ + sessionId: params.sessionId, + sessionFile: params.sessionFile, + sessionKey: params.sessionKey, + agentId: params.agentId, + config: params.config, + }), hookCtx: { runId: params.runId, agentId: sessionAgentId, diff --git a/src/agents/cli-runner/session-history.test.ts b/src/agents/cli-runner/session-history.test.ts new file mode 100644 index 00000000000..f1beb4d7db7 --- /dev/null +++ b/src/agents/cli-runner/session-history.test.ts @@ -0,0 +1,152 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { CURRENT_SESSION_VERSION } from "@mariozechner/pi-coding-agent"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { + loadCliSessionHistoryMessages, + MAX_CLI_SESSION_HISTORY_FILE_BYTES, + MAX_CLI_SESSION_HISTORY_MESSAGES, +} from "./session-history.js"; + +function createSessionTranscript(params: { + rootDir: string; + sessionId: string; + agentId?: string; + filePath?: string; + messages?: string[]; +}): string { + const sessionFile = + params.filePath ?? + path.join( + params.rootDir, + "agents", + params.agentId ?? "main", + "sessions", + `${params.sessionId}.jsonl`, + ); + fs.mkdirSync(path.dirname(sessionFile), { recursive: true }); + fs.writeFileSync( + sessionFile, + `${JSON.stringify({ + type: "session", + version: CURRENT_SESSION_VERSION, + id: params.sessionId, + timestamp: new Date(0).toISOString(), + cwd: params.rootDir, + })}\n`, + "utf-8", + ); + for (const [index, message] of (params.messages ?? []).entries()) { + fs.appendFileSync( + sessionFile, + `${JSON.stringify({ + type: "message", + id: `msg-${index}`, + parentId: index > 0 ? `msg-${index - 1}` : null, + timestamp: new Date(index + 1).toISOString(), + message: { + role: "user", + content: message, + timestamp: index + 1, + }, + })}\n`, + "utf-8", + ); + } + return sessionFile; +} + +describe("loadCliSessionHistoryMessages", () => { + afterEach(() => { + vi.unstubAllEnvs(); + }); + + it("reads the canonical session transcript instead of an arbitrary external path", () => { + const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-cli-state-")); + const outsideDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-cli-outside-")); + vi.stubEnv("OPENCLAW_STATE_DIR", stateDir); + createSessionTranscript({ + rootDir: stateDir, + sessionId: "session-test", + messages: ["expected history"], + }); + const outsideFile = createSessionTranscript({ + rootDir: outsideDir, + sessionId: "session-test", + filePath: path.join(outsideDir, "stolen.jsonl"), + messages: ["stolen history"], + }); + + try { + expect( + loadCliSessionHistoryMessages({ + sessionId: "session-test", + sessionFile: outsideFile, + sessionKey: "agent:main:main", + agentId: "main", + }), + ).toMatchObject([{ role: "user", content: "expected history" }]); + } finally { + fs.rmSync(stateDir, { recursive: true, force: true }); + fs.rmSync(outsideDir, { recursive: true, force: true }); + } + }); + + it("keeps only the newest bounded history window", () => { + const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-cli-state-")); + vi.stubEnv("OPENCLAW_STATE_DIR", stateDir); + const sessionFile = createSessionTranscript({ + rootDir: stateDir, + sessionId: "session-bounded", + messages: Array.from( + { length: MAX_CLI_SESSION_HISTORY_MESSAGES + 25 }, + (_, index) => `msg-${index}`, + ), + }); + + try { + const history = loadCliSessionHistoryMessages({ + sessionId: "session-bounded", + sessionFile, + sessionKey: "agent:main:main", + agentId: "main", + }); + expect(history).toHaveLength(MAX_CLI_SESSION_HISTORY_MESSAGES); + expect(history[0]).toMatchObject({ role: "user", content: "msg-25" }); + expect(history.at(-1)).toMatchObject({ + role: "user", + content: `msg-${MAX_CLI_SESSION_HISTORY_MESSAGES + 24}`, + }); + } finally { + fs.rmSync(stateDir, { recursive: true, force: true }); + } + }); + + it("drops oversized transcript files instead of loading them into hook payloads", () => { + const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-cli-state-")); + vi.stubEnv("OPENCLAW_STATE_DIR", stateDir); + const sessionFile = path.join( + stateDir, + "agents", + "main", + "sessions", + "session-oversized.jsonl", + ); + fs.mkdirSync(path.dirname(sessionFile), { recursive: true }); + fs.writeFileSync(sessionFile, "x".repeat(MAX_CLI_SESSION_HISTORY_FILE_BYTES + 1), "utf-8"); + + try { + expect( + loadCliSessionHistoryMessages({ + sessionId: "session-oversized", + sessionFile, + sessionKey: "agent:main:main", + agentId: "main", + }), + ).toEqual([]); + } finally { + fs.rmSync(stateDir, { recursive: true, force: true }); + } + }); +}); diff --git a/src/agents/cli-runner/session-history.ts b/src/agents/cli-runner/session-history.ts new file mode 100644 index 00000000000..437a2e39563 --- /dev/null +++ b/src/agents/cli-runner/session-history.ts @@ -0,0 +1,67 @@ +import fs from "node:fs"; +import { SessionManager } from "@mariozechner/pi-coding-agent"; +import { + resolveSessionFilePath, + resolveSessionFilePathOptions, +} from "../../config/sessions/paths.js"; +import type { OpenClawConfig } from "../../config/types.openclaw.js"; +import { resolveSessionAgentIds } from "../agent-scope.js"; + +export const MAX_CLI_SESSION_HISTORY_FILE_BYTES = 5 * 1024 * 1024; +export const MAX_CLI_SESSION_HISTORY_MESSAGES = 200; + +function resolveSafeCliSessionFile(params: { + sessionId: string; + sessionFile: string; + sessionKey?: string; + agentId?: string; + config?: OpenClawConfig; +}): string { + const { defaultAgentId, sessionAgentId } = resolveSessionAgentIds({ + sessionKey: params.sessionKey, + config: params.config, + agentId: params.agentId, + }); + return resolveSessionFilePath( + params.sessionId, + { sessionFile: params.sessionFile }, + resolveSessionFilePathOptions({ + agentId: sessionAgentId ?? defaultAgentId, + }), + ); +} + +export function loadCliSessionHistoryMessages(params: { + sessionId: string; + sessionFile: string; + sessionKey?: string; + agentId?: string; + config?: OpenClawConfig; +}): unknown[] { + try { + const sessionFile = resolveSafeCliSessionFile(params); + if (!fs.existsSync(sessionFile)) { + return []; + } + const stat = fs.statSync(sessionFile); + if (!stat.isFile() || stat.size > MAX_CLI_SESSION_HISTORY_FILE_BYTES) { + return []; + } + const entries = SessionManager.open(sessionFile).getEntries(); + const history: unknown[] = []; + for (let index = entries.length - 1; index >= 0; index -= 1) { + const entry = entries[index]; + if (entry?.type !== "message") { + continue; + } + history.push(entry.message as unknown); + if (history.length >= MAX_CLI_SESSION_HISTORY_MESSAGES) { + break; + } + } + history.reverse(); + return history; + } catch { + return []; + } +}