fix: harden flaky tests and cover native google thought signatures (#23457) (thanks @echoVic)
This commit is contained in:
@@ -50,6 +50,7 @@ Docs: https://docs.openclaw.ai
|
||||
- TUI/Status: request immediate renders after setting `sending`/`waiting` activity states so in-flight runs always show visible progress indicators instead of appearing idle until completion. (#21549) Thanks @13Guinness.
|
||||
- TUI/Input: arm Ctrl+C exit timing when clearing non-empty composer text and add a SIGINT fallback path so double Ctrl+C exits remain responsive during active runs instead of requiring an extra press or appearing stuck. (#23407) Thanks @tinybluedev.
|
||||
- Agents/Fallbacks: treat JSON payloads with `type: "api_error"` + `"Internal server error"` as transient failover errors so Anthropic 500-style failures trigger model fallback. (#23193) Thanks @jarvis-lane.
|
||||
- Agents/Google: sanitize non-base64 `thought_signature`/`thoughtSignature` values from assistant replay transcripts for native Google Gemini requests while preserving valid signatures and tool-call order. (#23457) Thanks @echoVic.
|
||||
- Agents/Transcripts: validate assistant tool-call names (syntax/length + registered tool allowlist) before transcript persistence and during replay sanitization so malformed failover tool names no longer poison sessions with repeated provider HTTP 400 errors. (#23324) Thanks @johnsantry.
|
||||
- Agents/Compaction: strip stale assistant usage snapshots from pre-compaction turns when replaying history after a compaction summary so context-token estimation no longer reuses pre-compaction totals and immediately re-triggers destructive follow-up compactions. (#19127) Thanks @tedwatson.
|
||||
- Agents/Replies: emit a default completion acknowledgement (`✅ Done.`) when runs execute tools successfully but return no final assistant text, preventing silent no-reply turns after tool-only completions. (#22834) Thanks @Oldshue.
|
||||
|
||||
@@ -231,6 +231,72 @@ describe("sanitizeSessionHistory (google thinking)", () => {
|
||||
]);
|
||||
});
|
||||
|
||||
it("strips non-base64 thought signatures for native Google Gemini", async () => {
|
||||
const sessionManager = SessionManager.inMemory();
|
||||
const input = [
|
||||
{
|
||||
role: "user",
|
||||
content: "hi",
|
||||
},
|
||||
{
|
||||
role: "assistant",
|
||||
content: [
|
||||
{ type: "text", text: "hello", thought_signature: "msg_abc123" },
|
||||
{ type: "thinking", thinking: "ok", thought_signature: "c2ln" },
|
||||
{
|
||||
type: "toolCall",
|
||||
id: "call_1",
|
||||
name: "read",
|
||||
arguments: { path: "/tmp/foo" },
|
||||
thoughtSignature: '{"id":1}',
|
||||
},
|
||||
{
|
||||
type: "toolCall",
|
||||
id: "call_2",
|
||||
name: "read",
|
||||
arguments: { path: "/tmp/bar" },
|
||||
thoughtSignature: "c2ln",
|
||||
},
|
||||
],
|
||||
},
|
||||
] as unknown as AgentMessage[];
|
||||
|
||||
const out = await sanitizeSessionHistory({
|
||||
messages: input,
|
||||
modelApi: "google-generative-ai",
|
||||
provider: "google",
|
||||
modelId: "gemini-2.0-flash",
|
||||
sessionManager,
|
||||
sessionId: "session:google-gemini",
|
||||
});
|
||||
|
||||
const assistant = out.find((msg) => (msg as { role?: string }).role === "assistant") as {
|
||||
content?: Array<{
|
||||
type?: string;
|
||||
thought_signature?: string;
|
||||
thoughtSignature?: string;
|
||||
thinking?: string;
|
||||
}>;
|
||||
};
|
||||
expect(assistant.content).toEqual([
|
||||
{ type: "text", text: "hello" },
|
||||
{ type: "thinking", thinking: "ok", thought_signature: "c2ln" },
|
||||
{
|
||||
type: "toolCall",
|
||||
id: "call1",
|
||||
name: "read",
|
||||
arguments: { path: "/tmp/foo" },
|
||||
},
|
||||
{
|
||||
type: "toolCall",
|
||||
id: "call2",
|
||||
name: "read",
|
||||
arguments: { path: "/tmp/bar" },
|
||||
thoughtSignature: "c2ln",
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it("keeps mixed signed/unsigned thinking blocks for Google models", async () => {
|
||||
const sessionManager = SessionManager.inMemory();
|
||||
const input = [
|
||||
|
||||
@@ -130,7 +130,7 @@ beforeAll(async () => {
|
||||
workspaceDir = path.join(tempRoot, "workspace");
|
||||
await fs.mkdir(agentDir, { recursive: true });
|
||||
await fs.mkdir(workspaceDir, { recursive: true });
|
||||
}, 60_000);
|
||||
}, 180_000);
|
||||
|
||||
afterAll(async () => {
|
||||
if (!tempRoot) {
|
||||
|
||||
@@ -1,10 +1,11 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import "./test-helpers/fast-core-tools.js";
|
||||
import {
|
||||
getCallGatewayMock,
|
||||
getSessionsSpawnTool,
|
||||
setSessionsSpawnConfigOverride,
|
||||
} from "./openclaw-tools.subagents.sessions-spawn.test-harness.js";
|
||||
import { resetSubagentRegistryForTests } from "./subagent-registry.js";
|
||||
|
||||
const hookRunnerMocks = vi.hoisted(() => ({
|
||||
hasSubagentEndedHook: true,
|
||||
@@ -79,6 +80,7 @@ function mockAgentStartFailure() {
|
||||
|
||||
describe("sessions_spawn subagent lifecycle hooks", () => {
|
||||
beforeEach(() => {
|
||||
resetSubagentRegistryForTests();
|
||||
hookRunnerMocks.hasSubagentEndedHook = true;
|
||||
hookRunnerMocks.runSubagentSpawning.mockClear();
|
||||
hookRunnerMocks.runSubagentSpawned.mockClear();
|
||||
@@ -103,6 +105,10 @@ describe("sessions_spawn subagent lifecycle hooks", () => {
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
resetSubagentRegistryForTests();
|
||||
});
|
||||
|
||||
it("runs subagent_spawning and emits subagent_spawned with requester metadata", async () => {
|
||||
const tool = await getSessionsSpawnTool({
|
||||
agentSessionKey: "main",
|
||||
|
||||
@@ -19,6 +19,10 @@ describe("resolveTranscriptPolicy", () => {
|
||||
modelApi: "google-generative-ai",
|
||||
});
|
||||
expect(policy.sanitizeToolCallIds).toBe(true);
|
||||
expect(policy.sanitizeThoughtSignatures).toEqual({
|
||||
allowBase64Only: true,
|
||||
includeCamelCase: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("enables sanitizeToolCallIds for Mistral provider", () => {
|
||||
|
||||
@@ -104,6 +104,22 @@ async function writeCronJobs(storePath: string, jobs: CronJob[]) {
|
||||
await fs.writeFile(storePath, JSON.stringify({ version: 1, jobs }, null, 2), "utf-8");
|
||||
}
|
||||
|
||||
async function removeDirWithRetries(dir: string, attempts = 3) {
|
||||
let lastError: unknown;
|
||||
for (let i = 0; i < attempts; i += 1) {
|
||||
try {
|
||||
await fs.rm(dir, { recursive: true, force: true });
|
||||
return;
|
||||
} catch (err) {
|
||||
lastError = err;
|
||||
await new Promise((resolve) => setTimeout(resolve, 25 * (i + 1)));
|
||||
}
|
||||
}
|
||||
if (lastError) {
|
||||
throw lastError;
|
||||
}
|
||||
}
|
||||
|
||||
async function startCronForStore(params: {
|
||||
storePath: string;
|
||||
cronEnabled?: boolean;
|
||||
@@ -142,7 +158,7 @@ describe("Cron issue regressions", () => {
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await fs.rm(fixtureRoot, { recursive: true, force: true });
|
||||
await removeDirWithRetries(fixtureRoot);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
|
||||
@@ -36,8 +36,8 @@ describe("runCommandWithTimeout", () => {
|
||||
const result = await runCommandWithTimeout(
|
||||
[process.execPath, "-e", "setTimeout(() => {}, 120)"],
|
||||
{
|
||||
timeoutMs: 1_000,
|
||||
noOutputTimeoutMs: 35,
|
||||
timeoutMs: 3_000,
|
||||
noOutputTimeoutMs: 120,
|
||||
},
|
||||
);
|
||||
|
||||
@@ -70,7 +70,7 @@ describe("runCommandWithTimeout", () => {
|
||||
const result = await runCommandWithTimeout(
|
||||
[process.execPath, "-e", "setTimeout(() => {}, 120)"],
|
||||
{
|
||||
timeoutMs: 15,
|
||||
timeoutMs: 100,
|
||||
},
|
||||
);
|
||||
|
||||
|
||||
@@ -9,7 +9,7 @@ describe("process supervisor", () => {
|
||||
backendId: "test",
|
||||
mode: "child",
|
||||
argv: [process.execPath, "-e", 'process.stdout.write("ok")'],
|
||||
timeoutMs: 2_500,
|
||||
timeoutMs: 10_000,
|
||||
stdinMode: "pipe-closed",
|
||||
});
|
||||
const exit = await run.wait();
|
||||
@@ -25,8 +25,8 @@ describe("process supervisor", () => {
|
||||
backendId: "test",
|
||||
mode: "child",
|
||||
argv: [process.execPath, "-e", "setTimeout(() => {}, 120)"],
|
||||
timeoutMs: 1_000,
|
||||
noOutputTimeoutMs: 20,
|
||||
timeoutMs: 3_000,
|
||||
noOutputTimeoutMs: 100,
|
||||
stdinMode: "pipe-closed",
|
||||
});
|
||||
const exit = await run.wait();
|
||||
@@ -43,7 +43,7 @@ describe("process supervisor", () => {
|
||||
scopeKey: "scope:a",
|
||||
mode: "child",
|
||||
argv: [process.execPath, "-e", "setTimeout(() => {}, 120)"],
|
||||
timeoutMs: 1_000,
|
||||
timeoutMs: 3_000,
|
||||
stdinMode: "pipe-open",
|
||||
});
|
||||
|
||||
@@ -54,7 +54,7 @@ describe("process supervisor", () => {
|
||||
replaceExistingScope: true,
|
||||
mode: "child",
|
||||
argv: [process.execPath, "-e", 'process.stdout.write("new")'],
|
||||
timeoutMs: 2_500,
|
||||
timeoutMs: 10_000,
|
||||
stdinMode: "pipe-closed",
|
||||
});
|
||||
|
||||
@@ -72,7 +72,7 @@ describe("process supervisor", () => {
|
||||
backendId: "test",
|
||||
mode: "child",
|
||||
argv: [process.execPath, "-e", "setTimeout(() => {}, 120)"],
|
||||
timeoutMs: 1,
|
||||
timeoutMs: 25,
|
||||
stdinMode: "pipe-closed",
|
||||
});
|
||||
const exit = await run.wait();
|
||||
@@ -88,7 +88,7 @@ describe("process supervisor", () => {
|
||||
backendId: "test",
|
||||
mode: "child",
|
||||
argv: [process.execPath, "-e", 'process.stdout.write("streamed")'],
|
||||
timeoutMs: 2_500,
|
||||
timeoutMs: 10_000,
|
||||
stdinMode: "pipe-closed",
|
||||
captureOutput: false,
|
||||
onStdout: (chunk) => {
|
||||
|
||||
@@ -13,6 +13,7 @@ const SKIP_PATTERNS = [
|
||||
/[\\/](?:__tests__|tests)[\\/]/,
|
||||
/[\\/][^\\/]*test-helpers(?:\.[^\\/]+)?\.ts$/,
|
||||
];
|
||||
const QUICK_TMPDIR_JOIN_PATTERN = /\bpath\.join\s*\(\s*os\.tmpdir\s*\(\s*\)/;
|
||||
|
||||
function shouldSkip(relativePath: string): boolean {
|
||||
return SKIP_PATTERNS.some((pattern) => pattern.test(relativePath));
|
||||
@@ -146,6 +147,9 @@ describe("temp path guard", () => {
|
||||
continue;
|
||||
}
|
||||
const source = await fs.readFile(file, "utf-8");
|
||||
if (!QUICK_TMPDIR_JOIN_PATTERN.test(source)) {
|
||||
continue;
|
||||
}
|
||||
if (hasDynamicTmpdirJoin(source, relativePath)) {
|
||||
offenders.push(relativePath);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user