mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 07:20:43 +00:00
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
This commit is contained in:
@@ -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<Array<unknown>>;
|
||||
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", () => {
|
||||
|
||||
@@ -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<EmbeddedPiRunResult> {
|
||||
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?.();
|
||||
|
||||
@@ -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");
|
||||
|
||||
@@ -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<typeof prepareDeps>): 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,
|
||||
|
||||
152
src/agents/cli-runner/session-history.test.ts
Normal file
152
src/agents/cli-runner/session-history.test.ts
Normal file
@@ -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 });
|
||||
}
|
||||
});
|
||||
});
|
||||
67
src/agents/cli-runner/session-history.ts
Normal file
67
src/agents/cli-runner/session-history.ts
Normal file
@@ -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 [];
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user