mirror of
https://github.com/Tencent/BrowserSkill.git
synced 2026-09-10 13:01:23 +08:00
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.
This commit is contained in:
@@ -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. */
|
||||
|
||||
@@ -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));
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user