From 2de7c5c6ff2a9950b96ec6755f9c1e57ed99cf44 Mon Sep 17 00:00:00 2001
From: BB-fat <1056871944@qq.com>
Date: Thu, 13 Aug 2026 15:27:29 +0000
Subject: [PATCH] fix(dsh-plugin-browserskill): enforce the session cap before
 spawning

The concurrency cap was checked in registry.add() AFTER
`bsk session start` had already created the session, so a rejected
start leaked a live session outside the plugin's tracking (found by the
real-browser e2e: the rejected third session stayed in
`bsk session list` after dispose). Check capacity before spawning;
registry.add() keeps the same check as a backstop.
---
 packages/dsh-plugin-browserskill/src/sessions.ts   | 14 +++++++++++---
 packages/dsh-plugin-browserskill/src/tools.ts      |  2 ++
 .../dsh-plugin-browserskill/tests/tools.test.ts    |  6 ++++--
 3 files changed, 17 insertions(+), 5 deletions(-)

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);
   });
 });