Skip to content

Commit fce00cc

Browse files
steipetesteipete
committed
fix(acp): ignore non-finite retention options
1 parent 2f8b1a8 commit fce00cc

5 files changed

Lines changed: 108 additions & 9 deletions

File tree

‎src/acp/event-ledger.test.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,39 @@ describe("ACP event ledger", () => {
7373
).resolves.toEqual({ complete: false, events: [] });
7474
});
7575

76+
it("falls back for non-finite event retention options", async () => {
77+
const ledger = createInMemoryAcpEventLedger({ maxEventsPerSession: Number.NaN });
78+
await ledger.startSession({
79+
sessionId: "session-1",
80+
sessionKey: "agent:main:work",
81+
cwd: "/work",
82+
complete: true,
83+
});
84+
await ledger.recordUpdate({
85+
sessionId: "session-1",
86+
sessionKey: "agent:main:work",
87+
update: {
88+
sessionUpdate: "agent_message_chunk",
89+
content: { type: "text", text: "First" },
90+
},
91+
});
92+
await ledger.recordUpdate({
93+
sessionId: "session-1",
94+
sessionKey: "agent:main:work",
95+
update: {
96+
sessionUpdate: "agent_message_chunk",
97+
content: { type: "text", text: "Second" },
98+
},
99+
});
100+
101+
await expect(
102+
ledger.readReplay({ sessionId: "session-1", sessionKey: "agent:main:work" }),
103+
).resolves.toMatchObject({
104+
complete: true,
105+
events: [{ seq: 1 }, { seq: 2 }],
106+
});
107+
});
108+
76109
it("persists file-backed replay state across ledger instances", async () => {
77110
await withTempDir({ prefix: "openclaw-acp-ledger-" }, async (dir) => {
78111
const filePath = path.join(dir, "acp", "event-ledger.json");

‎src/acp/event-ledger.ts‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { resolveStateDir } from "../config/paths.js";
55
import { withFileLock } from "../infra/file-lock.js";
66
import { readJsonFile, writeTextAtomic } from "../infra/json-files.js";
77
import { isRecord } from "../utils.js";
8+
import { resolveIntegerOption } from "./numeric-options.js";
89

910
const LEDGER_VERSION = 1;
1011
const DEFAULT_MAX_SESSIONS = 200;
@@ -103,14 +104,16 @@ function createEmptyStore(): LedgerStore {
103104

104105
function normalizeLedgerOptions(options: LedgerOptions = {}) {
105106
return {
106-
maxSessions: Math.max(1, Math.floor(options.maxSessions ?? DEFAULT_MAX_SESSIONS)),
107-
maxEventsPerSession: Math.max(
108-
1,
109-
Math.floor(options.maxEventsPerSession ?? DEFAULT_MAX_EVENTS_PER_SESSION),
107+
maxSessions: resolveIntegerOption(options.maxSessions, DEFAULT_MAX_SESSIONS, { min: 1 }),
108+
maxEventsPerSession: resolveIntegerOption(
109+
options.maxEventsPerSession,
110+
DEFAULT_MAX_EVENTS_PER_SESSION,
111+
{ min: 1 },
110112
),
111-
maxSerializedBytes: Math.max(
112-
1_024,
113-
Math.floor(options.maxSerializedBytes ?? DEFAULT_MAX_SERIALIZED_BYTES),
113+
maxSerializedBytes: resolveIntegerOption(
114+
options.maxSerializedBytes,
115+
DEFAULT_MAX_SERIALIZED_BYTES,
116+
{ min: 1_024 },
114117
),
115118
now: options.now ?? Date.now,
116119
};

‎src/acp/numeric-options.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
export function resolveIntegerOption(
2+
value: number | undefined,
3+
fallback: number,
4+
params: { min: number },
5+
): number {
6+
const candidate = typeof value === "number" && Number.isFinite(value) ? value : fallback;
7+
return Math.max(params.min, Math.floor(candidate));
8+
}

‎src/acp/session.test.ts‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,60 @@ describe("acp session manager", () => {
7575
expect(store.hasSession("existing")).toBe(true);
7676
});
7777

78+
it("falls back for non-finite idle TTL options", () => {
79+
const boundedStore = createInMemorySessionStore({
80+
maxSessions: 2,
81+
idleTtlMs: Number.NaN,
82+
now,
83+
});
84+
try {
85+
boundedStore.createSession({
86+
sessionId: "first",
87+
sessionKey: "acp:first",
88+
cwd: "/tmp",
89+
});
90+
advance(1);
91+
boundedStore.createSession({
92+
sessionId: "second",
93+
sessionKey: "acp:second",
94+
cwd: "/tmp",
95+
});
96+
97+
expect(boundedStore.hasSession("first")).toBe(true);
98+
expect(boundedStore.hasSession("second")).toBe(true);
99+
} finally {
100+
boundedStore.clearAllSessionsForTest();
101+
}
102+
});
103+
104+
it("falls back for non-finite max session options", () => {
105+
const boundedStore = createInMemorySessionStore({
106+
maxSessions: Number.NaN,
107+
idleTtlMs: 24 * 60 * 60 * 1_000,
108+
now,
109+
});
110+
try {
111+
for (let index = 0; index < 5_000; index += 1) {
112+
const session = boundedStore.createSession({
113+
sessionId: `session-${index}`,
114+
sessionKey: `acp:${index}`,
115+
cwd: "/tmp",
116+
});
117+
boundedStore.setActiveRun(session.sessionId, `run-${index}`, new AbortController());
118+
}
119+
120+
expect(() =>
121+
boundedStore.createSession({
122+
sessionId: "overflow",
123+
sessionKey: "acp:overflow",
124+
cwd: "/tmp",
125+
}),
126+
).toThrow(/session limit reached/i);
127+
} finally {
128+
boundedStore.clearAllSessionsForTest();
129+
}
130+
});
131+
78132
it("reaps idle sessions before enforcing the max session cap", () => {
79133
const boundedStore = createInMemorySessionStore({
80134
maxSessions: 1,

‎src/acp/session.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { randomUUID } from "node:crypto";
2+
import { resolveIntegerOption } from "./numeric-options.js";
23
import type { AcpSession } from "./types.js";
34

45
export type AcpSessionStore = {
@@ -28,8 +29,8 @@ const DEFAULT_MAX_SESSIONS = 5_000;
2829
const DEFAULT_IDLE_TTL_MS = 24 * 60 * 60 * 1_000;
2930

3031
export function createInMemorySessionStore(options: AcpSessionStoreOptions = {}): AcpSessionStore {
31-
const maxSessions = Math.max(1, options.maxSessions ?? DEFAULT_MAX_SESSIONS);
32-
const idleTtlMs = Math.max(1_000, options.idleTtlMs ?? DEFAULT_IDLE_TTL_MS);
32+
const maxSessions = resolveIntegerOption(options.maxSessions, DEFAULT_MAX_SESSIONS, { min: 1 });
33+
const idleTtlMs = resolveIntegerOption(options.idleTtlMs, DEFAULT_IDLE_TTL_MS, { min: 1_000 });
3334
const now = options.now ?? Date.now;
3435
const sessions = new Map<string, AcpSession>();
3536
const runIdToSessionId = new Map<string, string>();

0 commit comments

Comments
 (0)