Skip to content

Commit 459b3ad

Browse files
committed
clean up readonly functions
1 parent 14ae539 commit 459b3ad

File tree

3 files changed

+15
-101
lines changed

3 files changed

+15
-101
lines changed

stagehand/src/context.ts

Lines changed: 10 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import {
1313
} from "./logging.js";
1414
import {
1515
getSession,
16-
getSessionReadOnly,
1716
defaultSessionId,
1817
type BrowserSession
1918
} from "./sessionManager.js";
@@ -84,7 +83,6 @@ export class Context {
8483
this.stagehands.delete(sessionId);
8584
}
8685
}
87-
8886
public async getActivePage(): Promise<BrowserSession["page"] | null> {
8987
// Try to get page from Stagehand first (if available for this session)
9088
const stagehand = this.stagehands.get(this.currentSessionId);
@@ -94,79 +92,21 @@ export class Context {
9492

9593
// Fallback to session manager
9694
const session = await getSession(this.currentSessionId, this.config);
97-
if (!session || !session.page || session.page.isClosed()) {
98-
try {
99-
const currentSession = await getSession(
100-
this.currentSessionId,
101-
this.config
102-
);
103-
if (
104-
!currentSession ||
105-
!currentSession.page ||
106-
currentSession.page.isClosed()
107-
) {
108-
return null;
109-
}
110-
return currentSession.page;
111-
} catch (refreshError) {
112-
return null;
113-
}
114-
}
115-
return session.page;
116-
}
117-
118-
// Will create a new default session if one doesn't exist
119-
public async getActiveBrowser(): Promise<BrowserSession["browser"] | null> {
120-
const session = await getSession(this.currentSessionId, this.config);
121-
if (!session || !session.browser || !session.browser.isConnected()) {
122-
try {
123-
const currentSession = await getSession(
124-
this.currentSessionId,
125-
this.config
126-
);
127-
if (
128-
!currentSession ||
129-
!currentSession.browser ||
130-
!currentSession.browser.isConnected()
131-
) {
132-
return null;
133-
}
134-
return currentSession.browser;
135-
} catch (refreshError) {
136-
return null;
137-
}
95+
if (session && session.page && !session.page.isClosed()) {
96+
return session.page;
13897
}
139-
return session.browser;
98+
99+
return null;
140100
}
141101

142-
/**
143-
* Get the active browser without triggering session creation.
144-
* This is a read-only operation used when we need to check for an existing browser
145-
* without side effects (e.g., during close operations).
146-
* @returns The browser if it exists and is connected, null otherwise
147-
*/
148-
public getActiveBrowserReadOnly(): BrowserSession["browser"] | null {
149-
const session = getSessionReadOnly(this.currentSessionId);
102+
public async getActiveBrowser(createIfMissing: boolean = true): Promise<BrowserSession["browser"] | null> {
103+
const session = await getSession(this.currentSessionId, this.config, createIfMissing);
150104
if (!session || !session.browser || !session.browser.isConnected()) {
151105
return null;
152106
}
153107
return session.browser;
154108
}
155109

156-
/**
157-
* Get the active page without triggering session creation.
158-
* This is a read-only operation used when we need to check for an existing page
159-
* without side effects.
160-
* @returns The page if it exists and is not closed, null otherwise
161-
*/
162-
public getActivePageReadOnly(): BrowserSession["page"] | null {
163-
const session = getSessionReadOnly(this.currentSessionId);
164-
if (!session || !session.page || session.page.isClosed()) {
165-
return null;
166-
}
167-
return session.page;
168-
}
169-
170110
async run(tool: any, args: any): Promise<CallToolResult> {
171111
try {
172112
log(`Executing tool: ${tool.schema.name} with args: ${JSON.stringify(args)}`, 'info');
@@ -203,6 +143,10 @@ export class Context {
203143
}
204144
}
205145

146+
/**
147+
* List resources
148+
* Documentation: https://modelcontextprotocol.io/docs/concepts/resources
149+
*/
206150
listResources() {
207151
return listResources();
208152
}

stagehand/src/sessionManager.ts

Lines changed: 4 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ const browsers = new Map<string, BrowserSession>();
2222
let defaultBrowserSession: BrowserSession | null = null;
2323

2424
// Define a specific ID for the default session
25-
export const defaultSessionId = "browserbase_session_main";
25+
export const defaultSessionId = `browserbase_session_main_${Date.now()}`;
2626

2727
// Keep track of the active session ID. Defaults to the main session.
2828
let activeSessionId: string = defaultSessionId;
@@ -277,9 +277,10 @@ export async function ensureDefaultSessionInternal(
277277
// Get a specific session by ID
278278
export async function getSession(
279279
sessionId: string,
280-
config: Config
280+
config: Config,
281+
createIfMissing: boolean = true
281282
): Promise<BrowserSession | null> {
282-
if (sessionId === defaultSessionId) {
283+
if (sessionId === defaultSessionId && createIfMissing) {
283284
try {
284285
return await ensureDefaultSessionInternal(config);
285286
} catch (error) {
@@ -324,37 +325,6 @@ export async function getSession(
324325
return sessionObj;
325326
}
326327

327-
/**
328-
* Get a session by ID without creating new sessions.
329-
* This is a read-only operation that never triggers session creation.
330-
* Used for operations like closing sessions where we don't want side effects.
331-
* @param sessionId The session ID to retrieve
332-
* @returns The session if it exists and is valid, null otherwise
333-
*/
334-
export function getSessionReadOnly(sessionId: string): BrowserSession | null {
335-
// Check if it's the default session
336-
if (sessionId === defaultSessionId && defaultBrowserSession) {
337-
// Only return if it's actually connected and valid
338-
if (defaultBrowserSession.browser.isConnected() && !defaultBrowserSession.page.isClosed()) {
339-
return defaultBrowserSession;
340-
}
341-
return null;
342-
}
343-
344-
// For non-default sessions, check the browsers map
345-
const sessionObj = browsers.get(sessionId);
346-
if (!sessionObj) {
347-
return null;
348-
}
349-
350-
// Validate the session is still active
351-
if (!sessionObj.browser.isConnected() || sessionObj.page.isClosed()) {
352-
return null;
353-
}
354-
355-
return sessionObj;
356-
}
357-
358328
/**
359329
* Clean up a session by removing it from tracking.
360330
* This is called after a browser is closed to ensure proper cleanup.

stagehand/src/tools/session.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ async function handleCloseSession(
144144
// Step 1: Attempt to get the active browser instance WITHOUT creating a new one
145145
try {
146146
// Use read-only version to avoid creating new sessions
147-
browser = context.getActiveBrowserReadOnly();
147+
browser = await context.getActiveBrowser(false);
148148
} catch (error: any) {
149149
process.stderr.write(
150150
`[tool.closeSession] Error retrieving active browser (session ID was ${previousSessionId || 'default/unknown'}): ${error.message || String(error)}`

0 commit comments

Comments
 (0)