Skip to content

Commit 7af30e0

Browse files
authored
perf(memory): bound renderer sessions, terminals and polling (#3073)
1 parent 646f66c commit 7af30e0

21 files changed

Lines changed: 1008 additions & 67 deletions

packages/core/src/archive/archiveOrchestration.test.ts

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,7 @@ class Harness {
1818
unpin: vi.fn().mockResolvedValue(undefined),
1919
togglePin: vi.fn().mockResolvedValue(undefined),
2020
navigateAwayFromTaskIfActive: vi.fn(),
21-
snapshotTerminalStates: vi.fn().mockReturnValue({}),
2221
clearTerminalStates: vi.fn(),
23-
restoreTerminalStates: vi.fn(),
2422
snapshotCommandCenter: vi
2523
.fn()
2624
.mockReturnValue({ index: -1, wasActive: false }),
@@ -103,6 +101,27 @@ describe("archiveTask", () => {
103101
expect(harness.list).toEqual([]);
104102
expect(harness.deps.togglePin).toHaveBeenCalledWith(TASK_ID);
105103
});
104+
105+
it("destroys terminals only after the archive succeeds", async () => {
106+
let clearedWhenArchiveCalled = true;
107+
harness.deps.archive = vi.fn().mockImplementation(async () => {
108+
clearedWhenArchiveCalled =
109+
vi.mocked(harness.deps.clearTerminalStates).mock.calls.length > 0;
110+
});
111+
112+
await archiveTask(TASK_ID, harness.deps);
113+
114+
expect(clearedWhenArchiveCalled).toBe(false);
115+
expect(harness.deps.clearTerminalStates).toHaveBeenCalledWith(TASK_ID);
116+
});
117+
118+
it("keeps terminals when archive fails", async () => {
119+
harness.deps.archive = vi.fn().mockRejectedValue(new Error("boom"));
120+
121+
await expect(archiveTask(TASK_ID, harness.deps)).rejects.toThrow("boom");
122+
123+
expect(harness.deps.clearTerminalStates).not.toHaveBeenCalled();
124+
});
106125
});
107126

108127
describe("archiveTasks", () => {

packages/core/src/archive/archiveOrchestration.ts

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,7 @@ export interface ArchiveOrchestrationDeps {
2727
unpin(taskId: string): Promise<void>;
2828
togglePin(taskId: string): Promise<void>;
2929
navigateAwayFromTaskIfActive(taskId: string): void;
30-
snapshotTerminalStates(taskId: string): Record<string, unknown>;
3130
clearTerminalStates(taskId: string): void;
32-
restoreTerminalStates(states: Record<string, unknown>): void;
3331
snapshotCommandCenter(taskId: string): { index: number; wasActive: boolean };
3432
removeFromCommandCenter(taskId: string): void;
3533
restoreCommandCenter(
@@ -69,11 +67,9 @@ export async function archiveTask(
6967
deps.navigateAwayFromTaskIfActive(taskId);
7068
}
7169

72-
const terminalStatesSnapshot = deps.snapshotTerminalStates(taskId);
7370
const commandCenterSnapshot = deps.snapshotCommandCenter(taskId);
7471

7572
await deps.unpin(taskId);
76-
deps.clearTerminalStates(taskId);
7773
deps.removeFromCommandCenter(taskId);
7874

7975
await deps.cache.cancelPathFilter();
@@ -101,6 +97,9 @@ export async function archiveTask(
10197
try {
10298
await deps.disconnectFromTask(taskId);
10399
await deps.archive(taskId);
100+
// Destroying terminals is irreversible, so it waits for the archive to
101+
// commit; a failed archive keeps its live terminals.
102+
deps.clearTerminalStates(taskId);
104103
// Non-optimistic flows keep the row visible during the request, then remove
105104
// it the moment the archive succeeds.
106105
if (!optimistic) {
@@ -115,9 +114,6 @@ export async function archiveTask(
115114
if (wasPinned) {
116115
await deps.togglePin(taskId);
117116
}
118-
if (Object.keys(terminalStatesSnapshot).length > 0) {
119-
deps.restoreTerminalStates(terminalStatesSnapshot);
120-
}
121117
if (commandCenterSnapshot.index !== -1) {
122118
deps.restoreCommandCenter(taskId, commandCenterSnapshot);
123119
}
Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
import type { AgentSession } from "@posthog/shared";
2+
import { describe, expect, it } from "vitest";
3+
import { getCellCount, type LayoutPreset } from "../command-center/grid";
4+
import {
5+
isSessionIdle,
6+
MAX_CONNECTED_SESSIONS,
7+
selectSessionsToEvict,
8+
} from "./sessionEviction";
9+
10+
function makeSession(overrides: Partial<AgentSession>): AgentSession {
11+
return {
12+
taskRunId: `run-${overrides.taskId}`,
13+
status: "connected",
14+
isPromptPending: false,
15+
pendingPermissions: new Map(),
16+
messageQueue: [],
17+
startedAt: 0,
18+
...overrides,
19+
} as AgentSession;
20+
}
21+
22+
describe("isSessionIdle", () => {
23+
it.each([
24+
["connected idle local session", {}, true],
25+
["connecting session", { status: "connecting" as const }, false],
26+
["pending prompt", { isPromptPending: true }, false],
27+
["compacting session", { isCompacting: true }, false],
28+
["handoff in progress", { handoffInProgress: true }, false],
29+
[
30+
"pending permission",
31+
{ pendingPermissions: new Map([["p1", {} as never]]) },
32+
false,
33+
],
34+
[
35+
"queued messages",
36+
{ messageQueue: [{ id: "m1", content: "x", queuedAt: 0 }] },
37+
false,
38+
],
39+
[
40+
"running cloud session",
41+
{ isCloud: true, cloudStatus: "in_progress" as const },
42+
false,
43+
],
44+
[
45+
"queued cloud session",
46+
{ isCloud: true, cloudStatus: "queued" as const },
47+
false,
48+
],
49+
[
50+
"completed cloud session",
51+
{ isCloud: true, cloudStatus: "completed" as const },
52+
true,
53+
],
54+
["cloud session without status", { isCloud: true }, false],
55+
["disconnected local session", { status: "disconnected" as const }, true],
56+
["errored local session", { status: "error" as const }, true],
57+
])("%s -> %s", (_name, overrides, expected) => {
58+
expect(isSessionIdle(makeSession({ taskId: "t", ...overrides }))).toBe(
59+
expected,
60+
);
61+
});
62+
});
63+
64+
describe("selectSessionsToEvict", () => {
65+
const lastUsedAt = (session: AgentSession) => session.startedAt;
66+
67+
it.each([
68+
[
69+
"returns nothing under the budget",
70+
{
71+
sessions: [makeSession({ taskId: "a" }), makeSession({ taskId: "b" })],
72+
activeTaskId: "a",
73+
maxSessions: 3,
74+
},
75+
[],
76+
],
77+
[
78+
"evicts the least recently used idle sessions over the budget",
79+
{
80+
sessions: [
81+
makeSession({ taskId: "a", startedAt: 30 }),
82+
makeSession({ taskId: "b", startedAt: 10 }),
83+
makeSession({ taskId: "c", startedAt: 20 }),
84+
makeSession({ taskId: "d", startedAt: 40 }),
85+
],
86+
activeTaskId: "d",
87+
maxSessions: 3,
88+
},
89+
["b", "c"],
90+
],
91+
[
92+
"never evicts the active task or busy sessions",
93+
{
94+
sessions: [
95+
makeSession({ taskId: "active", startedAt: 1 }),
96+
makeSession({ taskId: "busy", startedAt: 2, isPromptPending: true }),
97+
makeSession({ taskId: "idle", startedAt: 3 }),
98+
],
99+
activeTaskId: "active",
100+
maxSessions: 2,
101+
},
102+
["idle"],
103+
],
104+
[
105+
"never evicts mounted tasks",
106+
{
107+
sessions: [
108+
makeSession({ taskId: "a", startedAt: 1 }),
109+
makeSession({ taskId: "b", startedAt: 2 }),
110+
makeSession({ taskId: "c", startedAt: 3 }),
111+
],
112+
activeTaskId: "c",
113+
protectedTaskIds: new Set(["a"]),
114+
maxSessions: 2,
115+
},
116+
["b"],
117+
],
118+
])("%s", (_name, params, expected) => {
119+
const evicted = selectSessionsToEvict({ ...params, lastUsedAt });
120+
expect(evicted.map((s) => s.taskId)).toEqual(expected);
121+
});
122+
});
123+
124+
describe("MAX_CONNECTED_SESSIONS", () => {
125+
it("stays above the largest Command Center grid so full layouts never evict", () => {
126+
// Record<LayoutPreset, ...> forces this list to grow with the union, so a
127+
// new larger preset breaks this test instead of silently churning cells.
128+
const allPresets: Record<LayoutPreset, true> = {
129+
"1x1": true,
130+
"2x1": true,
131+
"1x2": true,
132+
"2x2": true,
133+
"3x2": true,
134+
"3x3": true,
135+
};
136+
const largestGrid = Math.max(
137+
...(Object.keys(allPresets) as LayoutPreset[]).map(getCellCount),
138+
);
139+
140+
expect(MAX_CONNECTED_SESSIONS).toBeGreaterThan(largestGrid);
141+
});
142+
});
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
import type { AgentSession } from "@posthog/shared";
2+
import { isTerminalStatus } from "@posthog/shared/domain-types";
3+
4+
// Above the Command Center's 3x3 grid so fully-visible layouts never evict.
5+
export const MAX_CONNECTED_SESSIONS = 12;
6+
7+
export function isSessionIdle(session: AgentSession): boolean {
8+
if (session.status === "connecting") return false;
9+
if (session.isPromptPending) return false;
10+
if (session.isCompacting) return false;
11+
if (session.handoffInProgress) return false;
12+
if (session.pendingPermissions.size > 0) return false;
13+
if (session.messageQueue.length > 0) return false;
14+
if (session.isCloud) return isTerminalStatus(session.cloudStatus);
15+
return true;
16+
}
17+
18+
export function selectSessionsToEvict(params: {
19+
sessions: AgentSession[];
20+
activeTaskId: string;
21+
protectedTaskIds?: ReadonlySet<string>;
22+
lastUsedAt: (session: AgentSession) => number;
23+
maxSessions?: number;
24+
}): AgentSession[] {
25+
const { sessions, activeTaskId, protectedTaskIds, lastUsedAt } = params;
26+
const maxSessions = params.maxSessions ?? MAX_CONNECTED_SESSIONS;
27+
28+
// Reserves a slot for the incoming session even when a resume replaces an
29+
// existing one; deliberately over-evicts by one in that case.
30+
const excess = sessions.length - (maxSessions - 1);
31+
if (excess <= 0) return [];
32+
33+
return sessions
34+
.filter(
35+
(session) =>
36+
session.taskId !== activeTaskId &&
37+
!protectedTaskIds?.has(session.taskId) &&
38+
isSessionIdle(session),
39+
)
40+
.sort((a, b) => lastUsedAt(a) - lastUsedAt(b))
41+
.slice(0, excess);
42+
}

packages/core/src/sessions/sessionService.ts

Lines changed: 66 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ import {
7777
promptReferencesAbsoluteFolder,
7878
shellExecutesToContextBlocks,
7979
} from "./sessionEvents";
80+
import { selectSessionsToEvict } from "./sessionEviction";
8081
import { createBaseSession } from "./sessionFactory";
8182
import { type ParsedSessionLogs, parseSessionLogContent } from "./sessionLogs";
8283

@@ -513,6 +514,8 @@ export class SessionService {
513514
>();
514515
private localRepoPaths = new Map<string, string>();
515516
private localRecoveryAttempts = new Map<string, Promise<boolean>>();
517+
private sessionLastUsedAt = new Map<string, number>();
518+
private mountedTaskCounts = new Map<string, number>();
516519
/** Re-entrance guard for cloud queue dispatch (per taskId). */
517520
private dispatchingCloudQueues = new Set<string>();
518521
/** Coalesces deferred cloud queue flush timers (per taskId). */
@@ -610,6 +613,8 @@ export class SessionService {
610613
const { task } = params;
611614
const taskId = task.id;
612615
this.localRepoPaths.set(taskId, params.repoPath);
616+
this.sessionLastUsedAt.set(taskId, Date.now());
617+
void this.evictIdleSessions(taskId);
613618

614619
// Return existing connection promise if already connecting
615620
const existingPromise = this.connectingTasks.get(taskId);
@@ -1029,7 +1034,10 @@ export class SessionService {
10291034
}
10301035
}
10311036

1032-
private async teardownSession(taskRunId: string): Promise<void> {
1037+
private async teardownSession(
1038+
taskRunId: string,
1039+
opts?: { preserveResumeState?: boolean },
1040+
): Promise<void> {
10331041
const session = this.getSessionByRunId(taskRunId);
10341042

10351043
try {
@@ -1053,9 +1061,14 @@ export class SessionService {
10531061
if (session) {
10541062
this.localRepoPaths.delete(session.taskId);
10551063
this.localRecoveryAttempts.delete(session.taskId);
1064+
this.sessionLastUsedAt.delete(session.taskId);
1065+
}
1066+
if (!opts?.preserveResumeState) {
1067+
// Reconnect restores the model and permission mode from these; only a
1068+
// permanent disconnect (archive, delete, fresh session) may drop them.
1069+
this.d.adapterStore.removeAdapter(taskRunId);
1070+
this.d.removePersistedConfigOptions(taskRunId);
10561071
}
1057-
this.d.adapterStore.removeAdapter(taskRunId);
1058-
this.d.removePersistedConfigOptions(taskRunId);
10591072
}
10601073

10611074
/**
@@ -1358,6 +1371,51 @@ export class SessionService {
13581371
await this.teardownSession(session.taskRunId);
13591372
}
13601373

1374+
registerMountedTask(taskId: string): () => void {
1375+
this.mountedTaskCounts.set(
1376+
taskId,
1377+
(this.mountedTaskCounts.get(taskId) ?? 0) + 1,
1378+
);
1379+
this.sessionLastUsedAt.set(taskId, Date.now());
1380+
return () => {
1381+
const count = this.mountedTaskCounts.get(taskId) ?? 0;
1382+
if (count <= 1) {
1383+
this.mountedTaskCounts.delete(taskId);
1384+
} else {
1385+
this.mountedTaskCounts.set(taskId, count - 1);
1386+
}
1387+
this.sessionLastUsedAt.set(taskId, Date.now());
1388+
};
1389+
}
1390+
1391+
private async evictIdleSessions(activeTaskId: string): Promise<void> {
1392+
const toEvict = selectSessionsToEvict({
1393+
sessions: Object.values(this.d.store.getSessions()),
1394+
activeTaskId,
1395+
protectedTaskIds: new Set(this.mountedTaskCounts.keys()),
1396+
lastUsedAt: (session) =>
1397+
this.sessionLastUsedAt.get(session.taskId) ?? session.startedAt,
1398+
});
1399+
1400+
for (const session of toEvict) {
1401+
this.d.log.info("Evicting idle session to bound memory", {
1402+
taskId: session.taskId,
1403+
taskRunId: session.taskRunId,
1404+
});
1405+
this.sessionLastUsedAt.delete(session.taskId);
1406+
try {
1407+
await this.teardownSession(session.taskRunId, {
1408+
preserveResumeState: true,
1409+
});
1410+
} catch (error) {
1411+
this.d.log.error("Failed to evict idle session", {
1412+
taskId: session.taskId,
1413+
error,
1414+
});
1415+
}
1416+
}
1417+
}
1418+
13611419
// --- Subscription Management ---
13621420

13631421
/** Streamed events awaiting their frame flush, keyed by taskRunId. Order
@@ -1598,6 +1656,7 @@ export class SessionService {
15981656
this.connectingTasks.clear();
15991657
this.localRepoPaths.clear();
16001658
this.localRecoveryAttempts.clear();
1659+
this.sessionLastUsedAt.clear();
16011660
this.cloudPermissionRequestIds.clear();
16021661
this.liveTurnContent.clear();
16031662
this.cloudLogGapReconciler.clear();
@@ -4285,6 +4344,10 @@ export class SessionService {
42854344
} = params;
42864345

42874346
if (isCloud) {
4347+
// Local connects bound the session budget inside connectToTask; cloud
4348+
// watches would otherwise never trigger eviction.
4349+
this.sessionLastUsedAt.set(task.id, Date.now());
4350+
void this.evictIdleSessions(task.id);
42884351
return this.reconcileCloudConnection(
42894352
task,
42904353
cloudAuth,

0 commit comments

Comments
 (0)