diff --git a/packages/dsh-plugin-browserskill/src/sessions.ts b/packages/dsh-plugin-browserskill/src/sessions.ts index acf125a..b470f30 100644 --- a/packages/dsh-plugin-browserskill/src/sessions.ts +++ b/packages/dsh-plugin-browserskill/src/sessions.ts @@ -23,14 +23,22 @@ export class SessionRegistry { * @throws when the configured concurrency cap is already reached. */ add(session: TrackedSession): void { - if (!this.sessions.has(session.sessionId) && this.sessions.size >= this.maxSessions) { + if (!this.sessions.has(session.sessionId)) this.assertCapacity(); + this.sessions.set(session.sessionId, session); + this.currentId = session.sessionId; + } + + /** + * Throw when starting another session would exceed the concurrency cap. + * Call BEFORE spawning a new session so a rejected start never leaks one. + */ + assertCapacity(): void { + if (this.sessions.size >= this.maxSessions) { throw new Error( `session limit reached (${this.maxSessions} concurrent sessions); ` + "stop one with browser_session_stop before starting another", ); } - this.sessions.set(session.sessionId, session); - this.currentId = session.sessionId; } /** Forget a stopped session; falls back to the most recent remaining one. */ diff --git a/packages/dsh-plugin-browserskill/src/tools.ts b/packages/dsh-plugin-browserskill/src/tools.ts index 739bcc5..7ca2059 100644 --- a/packages/dsh-plugin-browserskill/src/tools.ts +++ b/packages/dsh-plugin-browserskill/src/tools.ts @@ -160,6 +160,8 @@ export function registerTools(deps: ToolDeps): void { if ((args.width === undefined) !== (args.height === undefined)) { throw new Error("width and height must be given together"); } + // Check the cap BEFORE spawning so a rejected start never leaks a session. + registry.assertCapacity(); const startArgs = ["session", "start"]; if (args.width !== undefined && args.height !== undefined) { startArgs.push("--width", String(args.width), "--height", String(args.height)); diff --git a/packages/dsh-plugin-browserskill/tests/tools.test.ts b/packages/dsh-plugin-browserskill/tests/tools.test.ts index 0073790..e234f68 100644 --- a/packages/dsh-plugin-browserskill/tests/tools.test.ts +++ b/packages/dsh-plugin-browserskill/tests/tools.test.ts @@ -226,8 +226,8 @@ describe("multi-session behavior", () => { await expect(snapshot?.execute({}, makeExec())).rejects.toThrow(/browser_session_start/); }); - it("enforces the configured session cap", async () => { - const { tools } = setup( + it("enforces the configured session cap before spawning", async () => { + const { tools, calls } = setup( { "session start": seq([START_REPLY("s1"), START_REPLY("s2")]) }, {}, 1, @@ -235,6 +235,8 @@ describe("multi-session behavior", () => { await startSession(tools); const start = tools.get("browser_session_start"); await expect(start?.execute({}, makeExec())).rejects.toThrow(/session limit/); + // A rejected start must not spawn another bsk session (no leaked session). + expect(calls.filter((c) => c.args.join(" ").startsWith("session start"))).toHaveLength(1); }); });