feat: implement session TTL auto-cleanup (#3250)
- Add SessionTTLManager to cleanup idle sessions (default 1h idle, 24h max age) - Fix memory leaks from accumulating webhook sessions - Add loadCombinedSessionStoreForGateway and resolveGatewaySessionStoreTarget helpers - Fix import errors and ensure proper exports in src/config/sessions.ts - Verify functionality with new tests in session-ttl-cleanup.test.ts Resolves #3250
This commit is contained in:
parent
6372242da7
commit
0f6fd7f2ca
@ -7,3 +7,4 @@ export * from "./sessions/session-key.js";
|
|||||||
export * from "./sessions/store.js";
|
export * from "./sessions/store.js";
|
||||||
export * from "./sessions/types.js";
|
export * from "./sessions/types.js";
|
||||||
export * from "./sessions/transcript.js";
|
export * from "./sessions/transcript.js";
|
||||||
|
export * from "./sessions/gateway.js";
|
||||||
|
|||||||
59
src/config/sessions/gateway.ts
Normal file
59
src/config/sessions/gateway.ts
Normal file
@ -0,0 +1,59 @@
|
|||||||
|
import { DEFAULT_AGENT_ID } from "../../routing/session-key.js";
|
||||||
|
import { resolveDefaultSessionStorePath } from "./paths.js";
|
||||||
|
import { loadSessionStore } from "./store.js";
|
||||||
|
import type { SessionEntry } from "./types.js";
|
||||||
|
|
||||||
|
type Config = {
|
||||||
|
agents?: { list?: Array<{ id?: string }> };
|
||||||
|
};
|
||||||
|
|
||||||
|
export function loadCombinedSessionStoreForGateway(cfg: Config): {
|
||||||
|
store: Record<string, SessionEntry>;
|
||||||
|
} {
|
||||||
|
const agents = cfg.agents?.list ?? [];
|
||||||
|
const agentIds = new Set<string>();
|
||||||
|
if (agents.length === 0) {
|
||||||
|
agentIds.add(DEFAULT_AGENT_ID);
|
||||||
|
} else {
|
||||||
|
for (const a of agents) {
|
||||||
|
if (a.id) agentIds.add(a.id);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
const combinedStore: Record<string, SessionEntry> = {};
|
||||||
|
|
||||||
|
for (const agentId of agentIds) {
|
||||||
|
const storePath = resolveDefaultSessionStorePath(agentId);
|
||||||
|
const store = loadSessionStore(storePath);
|
||||||
|
Object.assign(combinedStore, store);
|
||||||
|
}
|
||||||
|
|
||||||
|
return { store: combinedStore };
|
||||||
|
}
|
||||||
|
|
||||||
|
export function resolveGatewaySessionStoreTarget(params: { cfg: Config; key: string }): {
|
||||||
|
storePath: string;
|
||||||
|
canonicalKey: string;
|
||||||
|
} {
|
||||||
|
const { cfg, key } = params;
|
||||||
|
const agents = cfg.agents?.list ?? [];
|
||||||
|
const agentIds = new Set<string>();
|
||||||
|
if (agents.length === 0) {
|
||||||
|
agentIds.add(DEFAULT_AGENT_ID);
|
||||||
|
} else {
|
||||||
|
for (const a of agents) {
|
||||||
|
if (a.id) agentIds.add(a.id);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
for (const agentId of agentIds) {
|
||||||
|
const storePath = resolveDefaultSessionStorePath(agentId);
|
||||||
|
const store = loadSessionStore(storePath);
|
||||||
|
if (key in store) {
|
||||||
|
return { storePath, canonicalKey: key };
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
const defaultPath = resolveDefaultSessionStorePath(DEFAULT_AGENT_ID);
|
||||||
|
return { storePath: defaultPath, canonicalKey: key };
|
||||||
|
}
|
||||||
@ -34,6 +34,7 @@ import type { PluginServicesHandle } from "../plugins/services.js";
|
|||||||
import type { RuntimeEnv } from "../runtime.js";
|
import type { RuntimeEnv } from "../runtime.js";
|
||||||
import { runOnboardingWizard } from "../wizard/onboarding.js";
|
import { runOnboardingWizard } from "../wizard/onboarding.js";
|
||||||
import { startGatewayConfigReloader } from "./config-reload.js";
|
import { startGatewayConfigReloader } from "./config-reload.js";
|
||||||
|
import { SessionTTLManager } from "./session-ttl-cleanup.js";
|
||||||
import {
|
import {
|
||||||
getHealthCache,
|
getHealthCache,
|
||||||
getHealthVersion,
|
getHealthVersion,
|
||||||
@ -544,6 +545,9 @@ export async function startGatewayServer(
|
|||||||
watchPath: CONFIG_PATH,
|
watchPath: CONFIG_PATH,
|
||||||
});
|
});
|
||||||
|
|
||||||
|
const sessionTTLManager = new SessionTTLManager((cfgAtStart.session as any)?.ttl);
|
||||||
|
sessionTTLManager.start();
|
||||||
|
|
||||||
const close = createGatewayCloseHandler({
|
const close = createGatewayCloseHandler({
|
||||||
bonjourStop,
|
bonjourStop,
|
||||||
tailscaleCleanup,
|
tailscaleCleanup,
|
||||||
@ -571,6 +575,7 @@ export async function startGatewayServer(
|
|||||||
|
|
||||||
return {
|
return {
|
||||||
close: async (opts) => {
|
close: async (opts) => {
|
||||||
|
sessionTTLManager.stop();
|
||||||
if (diagnosticsEnabled) {
|
if (diagnosticsEnabled) {
|
||||||
stopDiagnosticHeartbeat();
|
stopDiagnosticHeartbeat();
|
||||||
}
|
}
|
||||||
|
|||||||
148
src/gateway/session-ttl-cleanup.test.ts
Normal file
148
src/gateway/session-ttl-cleanup.test.ts
Normal file
@ -0,0 +1,148 @@
|
|||||||
|
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
|
||||||
|
import { SessionTTLManager } from "./session-ttl-cleanup";
|
||||||
|
|
||||||
|
const { mockStoreResult, mockUpdateSessionStore } = vi.hoisted(() => ({
|
||||||
|
mockStoreResult: { store: {} as Record<string, any> },
|
||||||
|
mockUpdateSessionStore: vi.fn(),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock("../config/sessions.js", () => ({
|
||||||
|
loadCombinedSessionStoreForGateway: () => ({
|
||||||
|
store: mockStoreResult.store,
|
||||||
|
storePath: "mock-store.json5",
|
||||||
|
}),
|
||||||
|
resolveGatewaySessionStoreTarget: ({ key }: { key: string }) => ({
|
||||||
|
storePath: "mock-store.json5",
|
||||||
|
canonicalKey: key,
|
||||||
|
}),
|
||||||
|
updateSessionStore: (path: string, cb: any) => mockUpdateSessionStore(path, cb),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock("../config/config.js", () => ({
|
||||||
|
loadConfig: () => ({}),
|
||||||
|
}));
|
||||||
|
|
||||||
|
describe("SessionTTLManager", () => {
|
||||||
|
let manager: SessionTTLManager;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
mockUpdateSessionStore.mockClear();
|
||||||
|
for (const key in mockStoreResult.store) delete mockStoreResult.store[key];
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
manager?.stop();
|
||||||
|
vi.useRealTimers();
|
||||||
|
});
|
||||||
|
|
||||||
|
const createSession = (id: string, createdAt: number, updatedAt: number) => {
|
||||||
|
mockStoreResult.store[id] = { createdAt, updatedAt };
|
||||||
|
};
|
||||||
|
|
||||||
|
describe("Basic functionality", () => {
|
||||||
|
it("should not cleanup active sessions", async () => {
|
||||||
|
const now = Date.now();
|
||||||
|
createSession("session1", now - 1000, now - 500);
|
||||||
|
|
||||||
|
manager = new SessionTTLManager({ idle: 3600 });
|
||||||
|
await manager.cleanup();
|
||||||
|
|
||||||
|
expect(mockUpdateSessionStore).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should cleanup sessions exceeding idle time", async () => {
|
||||||
|
const now = Date.now();
|
||||||
|
createSession("idle-session", now - 7200000, now - 7200000);
|
||||||
|
|
||||||
|
manager = new SessionTTLManager({ idle: 3600 });
|
||||||
|
await manager.cleanup();
|
||||||
|
|
||||||
|
expect(mockUpdateSessionStore).toHaveBeenCalledWith("mock-store.json5", expect.any(Function));
|
||||||
|
|
||||||
|
const callback = mockUpdateSessionStore.mock.calls[0][1];
|
||||||
|
const params = { "idle-session": {} };
|
||||||
|
const result = callback(params);
|
||||||
|
expect(result).not.toHaveProperty("idle-session");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should cleanup sessions exceeding max age", async () => {
|
||||||
|
const now = Date.now();
|
||||||
|
createSession("old-session", now - 90000000, now - 1000);
|
||||||
|
|
||||||
|
manager = new SessionTTLManager({ idle: 3600, maxAge: 86400 });
|
||||||
|
await manager.cleanup();
|
||||||
|
|
||||||
|
expect(mockUpdateSessionStore).toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("Exclude patterns", () => {
|
||||||
|
it("should exclude main session", async () => {
|
||||||
|
const now = Date.now();
|
||||||
|
createSession("main", now - 100000000, now - 100000000);
|
||||||
|
|
||||||
|
manager = new SessionTTLManager({ idle: 3600, exclude: ["main"] });
|
||||||
|
await manager.cleanup();
|
||||||
|
|
||||||
|
expect(mockUpdateSessionStore).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should exclude sessions matching wildcard patterns", async () => {
|
||||||
|
const now = Date.now();
|
||||||
|
createSession("hook:github:reviewer:123", now - 10000000, now - 10000000);
|
||||||
|
createSession("hook:trello:doublon:456", now - 10000000, now - 10000000);
|
||||||
|
createSession("regular-session", now - 10000000, now - 10000000);
|
||||||
|
|
||||||
|
manager = new SessionTTLManager({
|
||||||
|
idle: 3600,
|
||||||
|
exclude: ["main", "hook:*", "persistent:*"],
|
||||||
|
});
|
||||||
|
await manager.cleanup();
|
||||||
|
|
||||||
|
expect(mockUpdateSessionStore).toHaveBeenCalledTimes(1);
|
||||||
|
const callback = mockUpdateSessionStore.mock.calls[0][1];
|
||||||
|
const result = callback({
|
||||||
|
"hook:github:reviewer:123": {},
|
||||||
|
"hook:trello:doublon:456": {},
|
||||||
|
"regular-session": {},
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(result).toHaveProperty("hook:github:reviewer:123");
|
||||||
|
expect(result).toHaveProperty("hook:trello:doublon:456");
|
||||||
|
expect(result).not.toHaveProperty("regular-session");
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("Automatic cleanup scheduling", () => {
|
||||||
|
it("should run cleanup at configured interval", () => {
|
||||||
|
manager = new SessionTTLManager({
|
||||||
|
idle: 3600,
|
||||||
|
cleanupInterval: 60,
|
||||||
|
});
|
||||||
|
|
||||||
|
const cleanupSpy = vi.spyOn(manager, "cleanup");
|
||||||
|
|
||||||
|
manager.start();
|
||||||
|
expect(cleanupSpy).toHaveBeenCalledTimes(1);
|
||||||
|
|
||||||
|
vi.advanceTimersByTime(60000);
|
||||||
|
expect(cleanupSpy).toHaveBeenCalledTimes(2);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should stop cleanup when stop() is called", () => {
|
||||||
|
manager = new SessionTTLManager({
|
||||||
|
idle: 3600,
|
||||||
|
cleanupInterval: 60,
|
||||||
|
});
|
||||||
|
|
||||||
|
const cleanupSpy = vi.spyOn(manager, "cleanup");
|
||||||
|
|
||||||
|
manager.start();
|
||||||
|
manager.stop();
|
||||||
|
|
||||||
|
vi.advanceTimersByTime(60000);
|
||||||
|
expect(cleanupSpy).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
177
src/gateway/session-ttl-cleanup.ts
Normal file
177
src/gateway/session-ttl-cleanup.ts
Normal file
@ -0,0 +1,177 @@
|
|||||||
|
import { loadConfig } from "../config/config.js";
|
||||||
|
import {
|
||||||
|
loadCombinedSessionStoreForGateway,
|
||||||
|
resolveGatewaySessionStoreTarget,
|
||||||
|
updateSessionStore,
|
||||||
|
} from "../config/sessions.js";
|
||||||
|
|
||||||
|
export interface SessionTTLConfig {
|
||||||
|
/**
|
||||||
|
* Idle time in seconds before a session is eligible for cleanup
|
||||||
|
* Default: 3600 (1 hour)
|
||||||
|
*/
|
||||||
|
idle?: number;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Maximum age in seconds regardless of activity
|
||||||
|
* Default: 86400 (24 hours)
|
||||||
|
*/
|
||||||
|
maxAge?: number;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* How often to run the cleanup process (in seconds)
|
||||||
|
* Default: 300 (5 minutes)
|
||||||
|
*/
|
||||||
|
cleanupInterval?: number;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Exclude certain session patterns from cleanup
|
||||||
|
* Example: ['main', 'persistent:*']
|
||||||
|
*/
|
||||||
|
exclude?: string[];
|
||||||
|
}
|
||||||
|
|
||||||
|
export class SessionTTLManager {
|
||||||
|
private config: Required<SessionTTLConfig>;
|
||||||
|
private cleanupTimer: NodeJS.Timeout | null = null;
|
||||||
|
private isRunning = false;
|
||||||
|
|
||||||
|
constructor(config: SessionTTLConfig = {}) {
|
||||||
|
this.config = {
|
||||||
|
idle: config.idle ?? 3600,
|
||||||
|
maxAge: config.maxAge ?? 86400,
|
||||||
|
cleanupInterval: config.cleanupInterval ?? 300,
|
||||||
|
exclude: config.exclude ?? ["main"],
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
start(): void {
|
||||||
|
if (this.cleanupTimer) {
|
||||||
|
console.warn("[SessionTTL] Cleanup already running");
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
console.log("[SessionTTL] Starting automatic session cleanup", {
|
||||||
|
idleSeconds: this.config.idle,
|
||||||
|
maxAgeSeconds: this.config.maxAge,
|
||||||
|
intervalSeconds: this.config.cleanupInterval,
|
||||||
|
});
|
||||||
|
|
||||||
|
this.cleanup().catch((err) => console.error("[SessionTTL] Cleanup failed on start:", err));
|
||||||
|
|
||||||
|
this.cleanupTimer = setInterval(() => {
|
||||||
|
this.cleanup().catch((err) => console.error("[SessionTTL] Cleanup failed:", err));
|
||||||
|
}, this.config.cleanupInterval * 1000);
|
||||||
|
}
|
||||||
|
|
||||||
|
stop(): void {
|
||||||
|
if (this.cleanupTimer) {
|
||||||
|
clearInterval(this.cleanupTimer);
|
||||||
|
this.cleanupTimer = null;
|
||||||
|
console.log("[SessionTTL] Stopped automatic session cleanup");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
async cleanup(): Promise<void> {
|
||||||
|
if (this.isRunning) {
|
||||||
|
console.log("[SessionTTL] Cleanup already in progress, skipping");
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
this.isRunning = true;
|
||||||
|
|
||||||
|
try {
|
||||||
|
const cfg = loadConfig();
|
||||||
|
const { store } = loadCombinedSessionStoreForGateway(cfg);
|
||||||
|
const now = Date.now();
|
||||||
|
const toDeleteByStore = new Map<string, string[]>();
|
||||||
|
|
||||||
|
let eligibleCount = 0;
|
||||||
|
|
||||||
|
for (const [key, session] of Object.entries(store)) {
|
||||||
|
if (this.shouldCleanup(key, session, now)) {
|
||||||
|
eligibleCount++;
|
||||||
|
// Resolve the physical store path for this session
|
||||||
|
const { storePath, canonicalKey } = resolveGatewaySessionStoreTarget({
|
||||||
|
cfg,
|
||||||
|
key,
|
||||||
|
});
|
||||||
|
|
||||||
|
if (!toDeleteByStore.has(storePath)) {
|
||||||
|
toDeleteByStore.set(storePath, []);
|
||||||
|
}
|
||||||
|
toDeleteByStore.get(storePath)!.push(canonicalKey);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (eligibleCount > 0) {
|
||||||
|
console.log(
|
||||||
|
`[SessionTTL] Found ${eligibleCount} sessions eligible for cleanup across ${toDeleteByStore.size} stores`,
|
||||||
|
);
|
||||||
|
|
||||||
|
for (const [storePath, keys] of toDeleteByStore.entries()) {
|
||||||
|
try {
|
||||||
|
await updateSessionStore(storePath, (params) => {
|
||||||
|
let deleted = 0;
|
||||||
|
for (const key of keys) {
|
||||||
|
if (params[key]) {
|
||||||
|
delete params[key];
|
||||||
|
deleted++;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if (deleted > 0) {
|
||||||
|
console.log(`[SessionTTL] Removed ${deleted} sessions from ${storePath}`);
|
||||||
|
}
|
||||||
|
return params;
|
||||||
|
});
|
||||||
|
} catch (err) {
|
||||||
|
console.error(`[SessionTTL] Failed to clean up store ${storePath}:`, err);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} catch (err) {
|
||||||
|
console.error("[SessionTTL] Cleanup failed:", err);
|
||||||
|
} finally {
|
||||||
|
this.isRunning = false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private shouldCleanup(
|
||||||
|
sessionKey: string,
|
||||||
|
session: { createdAt?: number; updatedAt?: number },
|
||||||
|
now: number,
|
||||||
|
): boolean {
|
||||||
|
if (this.isExcluded(sessionKey)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Default to 0 if missing, though typically they should exist.
|
||||||
|
// Use updatedAt as a proxy for both creation and activity if one is missing,
|
||||||
|
// assuming last write was last activity.
|
||||||
|
const createdAt = session.createdAt ?? session.updatedAt ?? 0;
|
||||||
|
const lastActivityAt = session.updatedAt ?? session.createdAt ?? 0;
|
||||||
|
|
||||||
|
const ageMs = now - createdAt;
|
||||||
|
const idleMs = now - lastActivityAt;
|
||||||
|
|
||||||
|
if (createdAt > 0 && ageMs > this.config.maxAge * 1000) {
|
||||||
|
// debug log only if verbose?
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (lastActivityAt > 0 && idleMs > this.config.idle * 1000) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
private isExcluded(sessionId: string): boolean {
|
||||||
|
return this.config.exclude.some((pattern) => {
|
||||||
|
if (pattern.endsWith("*")) {
|
||||||
|
const prefix = pattern.slice(0, -1);
|
||||||
|
return sessionId.startsWith(prefix);
|
||||||
|
}
|
||||||
|
return sessionId === pattern;
|
||||||
|
});
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
Reference in New Issue
Block a user