mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-16 11:41:08 +00:00
* fix(agents): preserve original task prompt on model fallback for new sessions * fix(agents): use dynamic transcript check for sessionHasHistory on fallback retry Address Greptile review feedback: replace the static !isNewSession flag with a dynamic sessionFileHasContent() check that reads the on-disk transcript before each fallback retry. This correctly handles the edge case where the primary model completes at least one assistant-response turn (flushing the user message to disk) before failing - the fallback now sends the recovery prompt instead of duplicating the original body. The !isNewSession short-circuit is kept as a fast path so existing sessions skip the file read entirely. * fix(agents): address security vulnerabilities in session fallback logic Fixes three medium-severity security issues identified by Aisle Security Analysis on PR #55632: - CWE-400: Unbounded session transcript read in sessionFileHasContent() - CWE-400: Symlink-following in sessionFileHasContent() - CWE-201: Sensitive prompt replay to a different fallback provider * fix(agents): use JSONL parsing for session history detection (CWE-703) Replace bounded byte-prefix substring matching in sessionFileHasContent() with line-by-line JSONL record parsing. The previous approach could miss an assistant message when the preceding user content exceeded the 256KB read limit, causing a false negative that blocks cross-provider fallback entirely. * fix(agents): preserve fallback prompt across providers --------- Co-authored-by: Ayaan Zaidi <hi@obviy.us>
160 lines
5.2 KiB
TypeScript
160 lines
5.2 KiB
TypeScript
import fs from "node:fs/promises";
|
|
import os from "node:os";
|
|
import path from "node:path";
|
|
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
|
import { resolveFallbackRetryPrompt, sessionFileHasContent } from "./attempt-execution.js";
|
|
|
|
describe("resolveFallbackRetryPrompt", () => {
|
|
const originalBody = "Summarize the quarterly earnings report and highlight key trends.";
|
|
|
|
it("returns original body on first attempt (isFallbackRetry=false)", () => {
|
|
expect(
|
|
resolveFallbackRetryPrompt({
|
|
body: originalBody,
|
|
isFallbackRetry: false,
|
|
}),
|
|
).toBe(originalBody);
|
|
});
|
|
|
|
it("returns recovery prompt for fallback retry with existing session history", () => {
|
|
expect(
|
|
resolveFallbackRetryPrompt({
|
|
body: originalBody,
|
|
isFallbackRetry: true,
|
|
sessionHasHistory: true,
|
|
}),
|
|
).toBe("Continue where you left off. The previous model attempt failed or timed out.");
|
|
});
|
|
|
|
it("preserves original body for fallback retry when session has no history (subagent spawn)", () => {
|
|
expect(
|
|
resolveFallbackRetryPrompt({
|
|
body: originalBody,
|
|
isFallbackRetry: true,
|
|
sessionHasHistory: false,
|
|
}),
|
|
).toBe(originalBody);
|
|
});
|
|
|
|
it("preserves original body for fallback retry when sessionHasHistory is undefined", () => {
|
|
expect(
|
|
resolveFallbackRetryPrompt({
|
|
body: originalBody,
|
|
isFallbackRetry: true,
|
|
}),
|
|
).toBe(originalBody);
|
|
});
|
|
|
|
it("returns original body on first attempt regardless of sessionHasHistory", () => {
|
|
expect(
|
|
resolveFallbackRetryPrompt({
|
|
body: originalBody,
|
|
isFallbackRetry: false,
|
|
sessionHasHistory: true,
|
|
}),
|
|
).toBe(originalBody);
|
|
|
|
expect(
|
|
resolveFallbackRetryPrompt({
|
|
body: originalBody,
|
|
isFallbackRetry: false,
|
|
sessionHasHistory: false,
|
|
}),
|
|
).toBe(originalBody);
|
|
});
|
|
|
|
it("preserves original body on fallback retry without history", () => {
|
|
expect(
|
|
resolveFallbackRetryPrompt({
|
|
body: originalBody,
|
|
isFallbackRetry: true,
|
|
sessionHasHistory: false,
|
|
}),
|
|
).toBe(originalBody);
|
|
});
|
|
});
|
|
|
|
describe("sessionFileHasContent", () => {
|
|
let tmpDir: string;
|
|
|
|
beforeEach(async () => {
|
|
tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "oc-test-"));
|
|
});
|
|
|
|
afterEach(async () => {
|
|
await fs.rm(tmpDir, { recursive: true, force: true });
|
|
});
|
|
|
|
it("returns false for undefined sessionFile", async () => {
|
|
expect(await sessionFileHasContent(undefined)).toBe(false);
|
|
});
|
|
|
|
it("returns false when session file does not exist", async () => {
|
|
expect(await sessionFileHasContent(path.join(tmpDir, "nonexistent.jsonl"))).toBe(false);
|
|
});
|
|
|
|
it("returns false when session file is empty", async () => {
|
|
const file = path.join(tmpDir, "empty.jsonl");
|
|
await fs.writeFile(file, "", "utf-8");
|
|
expect(await sessionFileHasContent(file)).toBe(false);
|
|
});
|
|
|
|
it("returns false when session file has only user message (no assistant flush)", async () => {
|
|
const file = path.join(tmpDir, "user-only.jsonl");
|
|
await fs.writeFile(
|
|
file,
|
|
'{"type":"session","id":"s1"}\n{"type":"message","message":{"role":"user","content":"hello"}}\n',
|
|
"utf-8",
|
|
);
|
|
expect(await sessionFileHasContent(file)).toBe(false);
|
|
});
|
|
|
|
it("returns true when session file has assistant message (flushed)", async () => {
|
|
const file = path.join(tmpDir, "with-assistant.jsonl");
|
|
await fs.writeFile(
|
|
file,
|
|
'{"type":"session","id":"s1"}\n{"type":"message","message":{"role":"user","content":"hello"}}\n{"type":"message","message":{"role":"assistant","content":"hi"}}\n',
|
|
"utf-8",
|
|
);
|
|
expect(await sessionFileHasContent(file)).toBe(true);
|
|
});
|
|
|
|
it("returns true when session file has spaced JSON (role : assistant)", async () => {
|
|
const file = path.join(tmpDir, "spaced.jsonl");
|
|
await fs.writeFile(
|
|
file,
|
|
'{"type":"message","message":{"role": "assistant","content":"hi"}}\n',
|
|
"utf-8",
|
|
);
|
|
expect(await sessionFileHasContent(file)).toBe(true);
|
|
});
|
|
|
|
it("returns true when assistant message appears after large user content", async () => {
|
|
const file = path.join(tmpDir, "large-user.jsonl");
|
|
// Create a user message whose JSON line exceeds 256KB to ensure the
|
|
// JSONL-based parser (CWE-703 fix) finds the assistant record that a
|
|
// naive byte-prefix approach would miss.
|
|
const bigContent = "x".repeat(300 * 1024);
|
|
const lines =
|
|
[
|
|
`{"type":"session","id":"s1"}`,
|
|
`{"type":"message","message":{"role":"user","content":"${bigContent}"}}`,
|
|
`{"type":"message","message":{"role":"assistant","content":"done"}}`,
|
|
].join("\n") + "\n";
|
|
await fs.writeFile(file, lines, "utf-8");
|
|
expect(await sessionFileHasContent(file)).toBe(true);
|
|
});
|
|
|
|
it("returns false when session file is a symbolic link", async () => {
|
|
const realFile = path.join(tmpDir, "real.jsonl");
|
|
await fs.writeFile(
|
|
realFile,
|
|
'{"type":"message","message":{"role":"assistant","content":"hi"}}\n',
|
|
"utf-8",
|
|
);
|
|
const link = path.join(tmpDir, "link.jsonl");
|
|
await fs.symlink(realFile, link);
|
|
expect(await sessionFileHasContent(link)).toBe(false);
|
|
});
|
|
});
|