Skip to content

Commit 6c71129

Browse files
committed
Make session containers sticky and harden warm pool
1 parent 15a6297 commit 6c71129

7 files changed

Lines changed: 468 additions & 110 deletions

File tree

src/index.ts

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,10 @@ async function main(): Promise<void> {
160160
const gatewayPort = envInt("OPENCLAW_GATEWAY_PORT", 18789);
161161
const readyTimeoutMs = envInt("OPENCLAW_READY_TIMEOUT_MS", 60_000);
162162
const runTimeoutMs = envInt("OPENCLAW_RUN_TIMEOUT_MS", 10 * 60_000);
163+
// Idle reap now applies only to sessions the caller explicitly marks
164+
// as reapable (today: ephemeral one-shot sessions). Sticky product
165+
// sessions keep their owned container alive between turns until
166+
// explicit teardown or process restart/adoption.
163167
const idleTimeoutMs = envInt("OPENCLAW_IDLE_TIMEOUT_MS", 10 * 60_000);
164168
const sweepIntervalMs = envInt("OPENCLAW_SWEEP_INTERVAL_MS", 60_000);
165169
// Warm pool is bounded so a host with many agent templates does not
@@ -246,18 +250,21 @@ async function main(): Promise<void> {
246250

247251
// Per-session container pool. isBusy closes over the session store so the
248252
// sweeper can skip containers whose session currently has a run in flight
249-
// — the pool itself has no store dependency. cleanupOnReap closes over
250-
// BOTH the store and the JSONL reader so it can tear down ephemeral
251-
// sessions (auto-created by keyless POST /v1/chat/completions) along with
252-
// their container. Called only on the idle-reap path; manual evictSession
253-
// and shutdown paths preserve session data.
253+
// — the pool itself has no store dependency. shouldReapSession marks only
254+
// ephemeral sessions as eligible for idle reap; sticky sessions keep their
255+
// container until explicit teardown. cleanupOnReap closes over BOTH the
256+
// store and the JSONL reader so it can tear down those ephemeral sessions
257+
// (auto-created by keyless POST /v1/chat/completions) along with their
258+
// container. Called only on the idle-reap path; manual evictSession and
259+
// shutdown paths preserve session data.
254260
const pool = new SessionContainerPool(runtime, {
255261
idleTimeoutMs,
256262
readyTimeoutMs,
257263
sweepIntervalMs,
258264
maxWarmContainers,
259265
warmIdleTimeoutMs,
260266
isBusy: (sessionId) => store.sessions.get(sessionId)?.status === "running",
267+
shouldReapSession: (sessionId) => store.sessions.get(sessionId)?.ephemeral === true,
261268
cleanupOnReap: async (sessionId) => {
262269
const session = store.sessions.get(sessionId);
263270
if (!session?.ephemeral) return;

src/orchestrator/router.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -465,6 +465,20 @@ export class AgentRouter {
465465
await this.pool.dropWarmForAgent(agentId);
466466
}
467467

468+
/**
469+
* Tear down any live runtime resources for a session without deleting the
470+
* session metadata itself. Used by DELETE /v1/sessions so the container,
471+
* queue, pending approvals, and persistent session↔container mapping do
472+
* not outlive the session row.
473+
*/
474+
async disposeSessionRuntime(sessionId: string): Promise<void> {
475+
this.cancelledDuringAcquire.delete(sessionId);
476+
this.queue.clear(sessionId);
477+
this.pendingApprovals.delete(sessionId);
478+
this.clearApprovalSubscriptions(sessionId);
479+
await this.pool.evictSession(sessionId);
480+
}
481+
468482
/**
469483
* Post a user.message to an existing session. Behavior depends on
470484
* session status:

src/orchestrator/server.test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ function makeApp() {
1212
const routerCalls = {
1313
warmForAgent: [] as string[],
1414
dropWarmForAgent: [] as string[],
15+
disposeSessionRuntime: [] as string[],
1516
};
1617

1718
const events = {
@@ -67,6 +68,10 @@ function makeApp() {
6768
routerCalls.dropWarmForAgent.push(agentId);
6869
return;
6970
},
71+
async disposeSessionRuntime(sessionId: string) {
72+
routerCalls.disposeSessionRuntime.push(sessionId);
73+
return;
74+
},
7075
async runEvent(args: { sessionId: string; content: string }) {
7176
const started = store.sessions.beginRun(args.sessionId);
7277
if (!started) {
@@ -338,4 +343,22 @@ describe("session ownership in the HTTP API", () => {
338343
expect(routerCalls.warmForAgent).toContain(agent.agentId);
339344
expect(store.agents.get(agent.agentId)?.model).toBe("openai/gpt-5.4");
340345
});
346+
347+
it("evicts live runtime state before deleting a session", async () => {
348+
const { app, store, routerCalls } = makeApp();
349+
const agent = createAgent(store);
350+
const session = store.sessions.create({
351+
agentId: agent.agentId,
352+
userId: null,
353+
});
354+
355+
const res = await req(app, `/v1/sessions/${session.sessionId}`, {
356+
method: "DELETE",
357+
token: "admin-secret",
358+
});
359+
360+
expect(res.status).toBe(200);
361+
expect(routerCalls.disposeSessionRuntime).toEqual([session.sessionId]);
362+
expect(store.sessions.get(session.sessionId)).toBeUndefined();
363+
});
341364
});

src/orchestrator/server.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -677,6 +677,9 @@ export function buildApp(deps: ServerDeps): Hono {
677677
outcome: archived ? "ok" : "agent_not_found",
678678
});
679679
if (!archived) return c.json({ error: "agent_not_found" }, 404);
680+
void deps.router.dropWarmForAgent(agentId).catch((err) => {
681+
log.warn({ err, agent_id: agentId }, "drop-warm-for-agent after archive failed (non-fatal)");
682+
});
680683
return c.json(agentResponse(archived));
681684
});
682685

@@ -1036,7 +1039,7 @@ export function buildApp(deps: ServerDeps): Hono {
10361039
return c.json(sessionResponse(session, deps.events, deps.sessionContainers));
10371040
});
10381041

1039-
app.delete("/v1/sessions/:sessionId", (c) => {
1042+
app.delete("/v1/sessions/:sessionId", async (c) => {
10401043
const sessionId = c.req.param("sessionId");
10411044
const session = getScopedSession(c, sessionId);
10421045
if (!session) {
@@ -1047,8 +1050,19 @@ export function buildApp(deps: ServerDeps): Hono {
10471050
});
10481051
return c.json({ error: "session_not_found" }, 404);
10491052
}
1050-
// Drop the Pi JSONL + sessions.json entry on disk first, then the
1051-
// orchestrator-side metadata row.
1053+
// Tear down the live runtime first so sticky session containers do
1054+
// not outlive their session row. Only once the container is gone do
1055+
// we delete the persisted JSONL / metadata.
1056+
try {
1057+
await deps.router.disposeSessionRuntime(sessionId);
1058+
} catch (err) {
1059+
writeAudit(deps.audit, c, {
1060+
action: "session.delete",
1061+
target: sessionId,
1062+
outcome: err instanceof RouterError ? err.code : "error",
1063+
});
1064+
return handleRouterError(err, c);
1065+
}
10521066
deps.events.deleteBySession(session.agentId, session.sessionId);
10531067
deps.sessions.delete(sessionId);
10541068
writeAudit(deps.audit, c, {

src/runtime/gateway-ws.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,10 @@ export class GatewayWebSocketClient {
105105

106106
constructor(private readonly cfg: GatewayWsConfig) {}
107107

108+
isConnected(): boolean {
109+
return this.connected && !this.closed;
110+
}
111+
108112
/**
109113
* Subscribe to gateway broadcast events. Returns an unsubscribe function.
110114
* Used by the orchestrator to listen for `plugin.approval.requested`

src/runtime/pool.test.ts

Lines changed: 120 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,20 @@ vi.mock("./gateway-ws.js", () => {
99
const instances: FakeWs[] = [];
1010
class FakeWs {
1111
readonly closeCount = { n: 0 };
12+
private connected = false;
1213
constructor(public readonly cfg: unknown) {
1314
instances.push(this);
1415
}
1516
async connect(): Promise<void> {
16-
/* noop */
17+
this.connected = true;
1718
}
1819
async close(): Promise<void> {
20+
this.connected = false;
1921
this.closeCount.n += 1;
2022
}
23+
isConnected(): boolean {
24+
return this.connected;
25+
}
2126
async abort(): Promise<void> {
2227
/* noop */
2328
}
@@ -413,6 +418,39 @@ describe("SessionContainerPool.warmForAgent", () => {
413418
expect(runtime.stopped.has("cnt_1")).toBe(true);
414419
await pool.shutdown();
415420
});
421+
422+
it("does not exceed maxWarmContainers when different agents warm concurrently", async () => {
423+
const { pool, runtime } = makePool({ maxWarmContainers: 1 });
424+
runtime.readyDelayMs = 25;
425+
426+
await Promise.all([
427+
pool.warmForAgent("agt_a", baseSpawnOptions("warm-a")),
428+
pool.warmForAgent("agt_b", baseSpawnOptions("warm-b")),
429+
]);
430+
431+
expect(runtime.calls.filter((c) => c.kind === "spawn")).toHaveLength(1);
432+
await pool.shutdown();
433+
});
434+
435+
it("cancels an inflight warm when dropWarmForAgent is called", async () => {
436+
const { pool, runtime } = makePool();
437+
runtime.readyDelayMs = 25;
438+
439+
const warming = pool.warmForAgent("agt_x", baseSpawnOptions());
440+
await new Promise((r) => setImmediate(r));
441+
await pool.dropWarmForAgent("agt_x");
442+
await warming;
443+
444+
const c = await pool.acquireForSession({
445+
sessionId: "ses_after_drop",
446+
spawnOptions: baseSpawnOptions(),
447+
agentId: "agt_x",
448+
});
449+
450+
expect(c.id).toBe("cnt_2");
451+
expect(runtime.stopped.has("cnt_1")).toBe(true);
452+
await pool.shutdown();
453+
});
416454
});
417455

418456
describe("SessionContainerPool.evictSession", () => {
@@ -501,6 +539,29 @@ describe("SessionContainerPool.reapIdle", () => {
501539
await pool.shutdown();
502540
});
503541

542+
it("keeps sticky sessions alive when shouldReapSession returns false", async () => {
543+
const { pool, runtime } = makePool({
544+
idleTimeoutMs: 50,
545+
shouldReapSession: (id) => id === "ses_ephemeral",
546+
});
547+
await pool.acquireForSession({
548+
sessionId: "ses_sticky",
549+
spawnOptions: baseSpawnOptions(),
550+
});
551+
await pool.acquireForSession({
552+
sessionId: "ses_ephemeral",
553+
spawnOptions: baseSpawnOptions(),
554+
});
555+
await new Promise((r) => setTimeout(r, 80));
556+
// @ts-expect-error — private reapIdle
557+
await pool.reapIdle();
558+
const sessionsLeft = pool.snapshot().map((e) => e.sessionId);
559+
expect(sessionsLeft).toEqual(["ses_sticky"]);
560+
expect(runtime.stopped.has("cnt_1")).toBe(false);
561+
expect(runtime.stopped.has("cnt_2")).toBe(true);
562+
await pool.shutdown();
563+
});
564+
504565
it("reaps warm containers past warmIdleTimeoutMs", async () => {
505566
const { pool, runtime } = makePool({
506567
idleTimeoutMs: 10 * 60_000,
@@ -534,6 +595,18 @@ describe("SessionContainerPool.shutdown", () => {
534595
expect(runtime.stopped.has("cnt_1")).toBe(true);
535596
expect(runtime.stopped.has("cnt_2")).toBe(true);
536597
});
598+
599+
it("waits for inflight warm boots and stops them during shutdown", async () => {
600+
const { pool, runtime } = makePool();
601+
runtime.readyDelayMs = 25;
602+
603+
const warming = pool.warmForAgent("agt_x", baseSpawnOptions());
604+
await new Promise((r) => setImmediate(r));
605+
await pool.shutdown();
606+
await warming;
607+
608+
expect(runtime.stopped.has("cnt_1")).toBe(true);
609+
});
537610
});
538611

539612
// --------------------------------------------------------------------
@@ -774,4 +847,50 @@ describe("SessionContainerPool — networking: limited", () => {
774847
expect(spawnsAfter - spawnsBefore).toBe(2);
775848
await pool.shutdown();
776849
});
850+
851+
it("falls back to a cold spawn when a claimed warm container is no longer ready", async () => {
852+
const { pool, runtime } = makePool();
853+
await pool.warmForAgent("agt_x", baseSpawnOptions());
854+
const baseWaitForReady = runtime.waitForReady.bind(runtime);
855+
runtime.waitForReady = async (container, timeoutMs) => {
856+
if (container.id === "cnt_1") {
857+
throw new Error("warm container is dead");
858+
}
859+
return baseWaitForReady(container, timeoutMs);
860+
};
861+
862+
const c = await pool.acquireForSession({
863+
sessionId: "ses_claim",
864+
spawnOptions: baseSpawnOptions(),
865+
agentId: "agt_x",
866+
});
867+
868+
expect(c.id).toBe("cnt_2");
869+
expect(runtime.stopped.has("cnt_1")).toBe(true);
870+
await pool.shutdown();
871+
});
872+
873+
it("cold-respawns when the existing active container fails the reuse probe", async () => {
874+
const { pool, runtime } = makePool();
875+
const first = await pool.acquireForSession({
876+
sessionId: "ses_live",
877+
spawnOptions: baseSpawnOptions(),
878+
});
879+
const baseWaitForReady = runtime.waitForReady.bind(runtime);
880+
runtime.waitForReady = async (container, timeoutMs) => {
881+
if (container.id === first.id) {
882+
throw new Error("active container is dead");
883+
}
884+
return baseWaitForReady(container, timeoutMs);
885+
};
886+
887+
const second = await pool.acquireForSession({
888+
sessionId: "ses_live",
889+
spawnOptions: baseSpawnOptions(),
890+
});
891+
892+
expect(second.id).toBe("cnt_2");
893+
expect(runtime.stopped.has(first.id)).toBe(true);
894+
await pool.shutdown();
895+
});
777896
});

0 commit comments

Comments
 (0)