diff --git a/src/agents/session-write-lock.test.ts b/src/agents/session-write-lock.test.ts index a8d09a0e1..09982b6c4 100644 --- a/src/agents/session-write-lock.test.ts +++ b/src/agents/session-write-lock.test.ts @@ -47,13 +47,34 @@ async function expectCurrentPidOwnsLock(params: { await lock.release(); } -async function expectActiveInProcessLockIsNotReclaimed(params?: { - legacyStarttime?: unknown; -}): Promise { +async function withTempSessionLockFile( + run: (params: { root: string; sessionFile: string; lockPath: string }) => Promise, +) { const root = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-lock-")); try { const sessionFile = path.join(root, "sessions.json"); - const lockPath = `${sessionFile}.lock`; + await run({ root, sessionFile, lockPath: `${sessionFile}.lock` }); + } finally { + await fs.rm(root, { recursive: true, force: true }); + } +} + +async function writeCurrentProcessLock(lockPath: string, extra?: Record) { + await fs.writeFile( + lockPath, + JSON.stringify({ + pid: process.pid, + createdAt: new Date().toISOString(), + ...extra, + }), + "utf8", + ); +} + +async function expectActiveInProcessLockIsNotReclaimed(params?: { + legacyStarttime?: unknown; +}): Promise { + await withTempSessionLockFile(async ({ sessionFile, lockPath }) => { const lock = await acquireSessionWriteLock({ sessionFile, timeoutMs: 500 }); const lockPayload = { pid: process.pid, @@ -70,9 +91,7 @@ async function expectActiveInProcessLockIsNotReclaimed(params?: { }), ).rejects.toThrow(/session file locked/); await lock.release(); - } finally { - await fs.rm(root, { recursive: true, force: true }); - } + }); } describe("acquireSessionWriteLock", () => { @@ -103,11 +122,7 @@ describe("acquireSessionWriteLock", () => { }); it("keeps the lock file until the last release", async () => { - const root = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-lock-")); - try { - const sessionFile = path.join(root, "sessions.json"); - const lockPath = `${sessionFile}.lock`; - + await withTempSessionLockFile(async ({ sessionFile, lockPath }) => { const lockA = await acquireSessionWriteLock({ sessionFile, timeoutMs: 500 }); const lockB = await acquireSessionWriteLock({ sessionFile, timeoutMs: 500 }); @@ -116,9 +131,7 @@ describe("acquireSessionWriteLock", () => { firstLock: lockA, secondLock: lockB, }); - } finally { - await fs.rm(root, { recursive: true, force: true }); - } + }); }); it("reclaims stale lock files", async () => { @@ -155,10 +168,7 @@ describe("acquireSessionWriteLock", () => { }); it("reclaims malformed lock files once they are old enough", async () => { - const root = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-lock-")); - try { - const sessionFile = path.join(root, "sessions.json"); - const lockPath = `${sessionFile}.lock`; + await withTempSessionLockFile(async ({ sessionFile, lockPath }) => { await fs.writeFile(lockPath, "{}", "utf8"); const staleDate = new Date(Date.now() - 2 * 60_000); await fs.utimes(lockPath, staleDate, staleDate); @@ -166,9 +176,7 @@ describe("acquireSessionWriteLock", () => { const lock = await acquireSessionWriteLock({ sessionFile, timeoutMs: 500, staleMs: 10_000 }); await lock.release(); await expect(fs.access(lockPath)).rejects.toThrow(); - } finally { - await fs.rm(root, { recursive: true, force: true }); - } + }); }); it("watchdog releases stale in-process locks", async () => { @@ -305,49 +313,24 @@ describe("acquireSessionWriteLock", () => { }); it("reclaims lock files with recycled PIDs", async () => { - const root = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-lock-")); - try { - const sessionFile = path.join(root, "sessions.json"); - const lockPath = `${sessionFile}.lock`; + await withTempSessionLockFile(async ({ sessionFile, lockPath }) => { // Write a lock with a live PID (current process) but a wrong starttime, // simulating PID recycling: the PID is alive but belongs to a different // process than the one that created the lock. - await fs.writeFile( - lockPath, - JSON.stringify({ - pid: process.pid, - createdAt: new Date().toISOString(), - starttime: 999_999_999, - }), - "utf8", - ); + await writeCurrentProcessLock(lockPath, { starttime: 999_999_999 }); await expectCurrentPidOwnsLock({ sessionFile, timeoutMs: 500 }); - } finally { - await fs.rm(root, { recursive: true, force: true }); - } + }); }); it("reclaims orphan lock files without starttime when PID matches current process", async () => { - const root = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-lock-")); - try { - const sessionFile = path.join(root, "sessions.json"); - const lockPath = `${sessionFile}.lock`; + await withTempSessionLockFile(async ({ sessionFile, lockPath }) => { // Simulate an old-format lock file left behind by a previous process // instance that reused the same PID (common in containers). - await fs.writeFile( - lockPath, - JSON.stringify({ - pid: process.pid, - createdAt: new Date().toISOString(), - }), - "utf8", - ); + await writeCurrentProcessLock(lockPath); await expectCurrentPidOwnsLock({ sessionFile, timeoutMs: 500 }); - } finally { - await fs.rm(root, { recursive: true, force: true }); - } + }); }); it("does not reclaim active in-process lock files without starttime", async () => { @@ -397,18 +380,13 @@ describe("acquireSessionWriteLock", () => { }); it("cleans up locks on exit", async () => { - const root = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-lock-")); - try { - const sessionFile = path.join(root, "sessions.json"); - const lockPath = `${sessionFile}.lock`; + await withTempSessionLockFile(async ({ sessionFile, lockPath }) => { await acquireSessionWriteLock({ sessionFile, timeoutMs: 500 }); process.emit("exit", 0); await expect(fs.access(lockPath)).rejects.toThrow(); - } finally { - await fs.rm(root, { recursive: true, force: true }); - } + }); }); it("keeps other signal listeners registered", () => { const keepAlive = () => {}; diff --git a/src/agents/tool-loop-detection.test.ts b/src/agents/tool-loop-detection.test.ts index 2a356f732..056c5286c 100644 --- a/src/agents/tool-loop-detection.test.ts +++ b/src/agents/tool-loop-detection.test.ts @@ -75,6 +75,48 @@ function createNoProgressPollFixture(sessionId: string) { }; } +function createReadNoProgressFixture() { + return { + toolName: "read", + params: { path: "/same.txt" }, + result: { + content: [{ type: "text", text: "same output" }], + details: { ok: true }, + }, + } as const; +} + +function createPingPongFixture() { + return { + state: createState(), + readParams: { path: "/a.txt" }, + listParams: { dir: "/workspace" }, + }; +} + +function detectLoopAfterRepeatedCalls(params: { + toolName: string; + toolParams: unknown; + result: unknown; + count: number; + config?: ToolLoopDetectionConfig; +}) { + const state = createState(); + recordRepeatedSuccessfulCalls({ + state, + toolName: params.toolName, + toolParams: params.toolParams, + result: params.result, + count: params.count, + }); + return detectToolCallLoop( + state, + params.toolName, + params.toolParams, + params.config ?? enabledLoopDetectionConfig, + ); +} + function recordSuccessfulPingPongCalls(params: { state: SessionState; readParams: { path: string }; @@ -258,18 +300,13 @@ describe("tool-loop-detection", () => { }); it("keeps generic loops warn-only below global breaker threshold", () => { - const state = createState(); - const params = { path: "/same.txt" }; - const result = { - content: [{ type: "text", text: "same output" }], - details: { ok: true }, - }; - - for (let i = 0; i < CRITICAL_THRESHOLD; i += 1) { - recordSuccessfulCall(state, "read", params, result, i); - } - - const loopResult = detectToolCallLoop(state, "read", params, enabledLoopDetectionConfig); + const fixture = createReadNoProgressFixture(); + const loopResult = detectLoopAfterRepeatedCalls({ + toolName: fixture.toolName, + toolParams: fixture.params, + result: fixture.result, + count: CRITICAL_THRESHOLD, + }); expect(loopResult.stuck).toBe(true); if (loopResult.stuck) { expect(loopResult.level).toBe("warning"); @@ -344,17 +381,13 @@ describe("tool-loop-detection", () => { }); it("warns for known polling no-progress loops", () => { - const state = createState(); const { params, result } = createNoProgressPollFixture("sess-1"); - recordRepeatedSuccessfulCalls({ - state, + const loopResult = detectLoopAfterRepeatedCalls({ toolName: "process", toolParams: params, result, count: WARNING_THRESHOLD, }); - - const loopResult = detectToolCallLoop(state, "process", params, enabledLoopDetectionConfig); expect(loopResult.stuck).toBe(true); if (loopResult.stuck) { expect(loopResult.level).toBe("warning"); @@ -364,17 +397,13 @@ describe("tool-loop-detection", () => { }); it("blocks known polling no-progress loops at critical threshold", () => { - const state = createState(); const { params, result } = createNoProgressPollFixture("sess-1"); - recordRepeatedSuccessfulCalls({ - state, + const loopResult = detectLoopAfterRepeatedCalls({ toolName: "process", toolParams: params, result, count: CRITICAL_THRESHOLD, }); - - const loopResult = detectToolCallLoop(state, "process", params, enabledLoopDetectionConfig); expect(loopResult.stuck).toBe(true); if (loopResult.stuck) { expect(loopResult.level).toBe("critical"); @@ -400,18 +429,13 @@ describe("tool-loop-detection", () => { }); it("blocks any tool with global no-progress breaker at 30", () => { - const state = createState(); - const params = { path: "/same.txt" }; - const result = { - content: [{ type: "text", text: "same output" }], - details: { ok: true }, - }; - - for (let i = 0; i < GLOBAL_CIRCUIT_BREAKER_THRESHOLD; i += 1) { - recordSuccessfulCall(state, "read", params, result, i); - } - - const loopResult = detectToolCallLoop(state, "read", params, enabledLoopDetectionConfig); + const fixture = createReadNoProgressFixture(); + const loopResult = detectLoopAfterRepeatedCalls({ + toolName: fixture.toolName, + toolParams: fixture.params, + result: fixture.result, + count: GLOBAL_CIRCUIT_BREAKER_THRESHOLD, + }); expect(loopResult.stuck).toBe(true); if (loopResult.stuck) { expect(loopResult.level).toBe("critical"); @@ -441,9 +465,7 @@ describe("tool-loop-detection", () => { }); it("blocks ping-pong alternating patterns at critical threshold", () => { - const state = createState(); - const readParams = { path: "/a.txt" }; - const listParams = { dir: "/workspace" }; + const { state, readParams, listParams } = createPingPongFixture(); recordSuccessfulPingPongCalls({ state, @@ -465,9 +487,7 @@ describe("tool-loop-detection", () => { }); it("does not block ping-pong at critical threshold when outcomes are progressing", () => { - const state = createState(); - const readParams = { path: "/a.txt" }; - const listParams = { dir: "/workspace" }; + const { state, readParams, listParams } = createPingPongFixture(); recordSuccessfulPingPongCalls({ state,