From f0437d9390e4bdcae959af3c6e9e1a45b53022fd Mon Sep 17 00:00:00 2001 From: Andrii Arsenin Date: Wed, 8 Apr 2026 11:10:08 +0300 Subject: [PATCH] refactor(browser): make sessionName optional, add selector filter, deduplicate session setup - sessionName is now optional in open/exec; skips session restore when omitted - add selector param to open: returns outerHTML or textContent of matched element - extract session restore logic into private setupSession() to remove duplication - replace per-exception instanceof checks with single HttpException base class check - embed label into InternalServerErrorException message instead of logging separately - update MCP tool schemas and HTTP DTOs to reflect optional sessionName and new selector - add integration tests: sessionless open/exec, selector, selector+readerMode --- src/browser/browser.controller.ts | 2 +- src/browser/browser.service.ts | 108 ++++++++++++------------------ src/browser/dto/exec.dto.ts | 7 +- src/browser/dto/open.dto.ts | 15 ++++- src/mcp/mcp.service.ts | 9 +-- test/__mocks__/jsdom.ts | 11 ++- test/browser.controller.spec.ts | 43 ++++++++++-- 7 files changed, 109 insertions(+), 86 deletions(-) diff --git a/src/browser/browser.controller.ts b/src/browser/browser.controller.ts index 2e6f13b..90b0521 100644 --- a/src/browser/browser.controller.ts +++ b/src/browser/browser.controller.ts @@ -30,7 +30,7 @@ export class BrowserController { @ApiResponse({ status: 404, description: 'Session not found' }) @ApiResponse({ status: 500, description: 'Browser automation failed' }) open(@Body() dto: OpenDto): Promise { - return this.browserService.open(dto.sessionName, dto.url, dto.readerMode ?? false); + return this.browserService.open(dto.sessionName, dto.url, dto.readerMode ?? false, dto.selector); } @Post('exec') diff --git a/src/browser/browser.service.ts b/src/browser/browser.service.ts index b7d9aec..2293d7b 100644 --- a/src/browser/browser.service.ts +++ b/src/browser/browser.service.ts @@ -1,11 +1,12 @@ import { Injectable, - BadRequestException, + HttpException, InternalServerErrorException, Logger, NotFoundException, } from '@nestjs/common'; import { chromium } from 'playwright'; +import type { BrowserContext } from 'playwright'; import { Readability } from '@mozilla/readability'; import { JSDOM } from 'jsdom'; import { SessionService } from '../session/session.service'; @@ -29,12 +30,12 @@ export class BrowserService { private readonly codeExecutor: CodeExecutorService, ) {} - async open(sessionName: string, url: string, readerMode = false): Promise { + private async setupSession(context: BrowserContext, sessionName: string | undefined): Promise { + if (!sessionName) return; const session = await this.sessionService.findBySessionName(sessionName); if (!session) { throw new NotFoundException(`Session not found: ${sessionName}`); } - let cookies: any[]; let localStorageData: Record; try { @@ -43,25 +44,33 @@ export class BrowserService { } catch (err) { throw new InternalServerErrorException('Failed to deserialize session data', { cause: err }); } + await context.addCookies(cookies); + await context.addInitScript((entries: Record) => { + for (const [k, v] of Object.entries(entries)) { + window.localStorage.setItem(k, v); + } + }, localStorageData); + } + private rethrow(err: unknown, label: string, operation: string): never { + if (err instanceof HttpException) { + throw err; + } + throw new InternalServerErrorException( + `[${label}] Browser ${operation} failed: ${(err as Error).message}`, + { cause: err }, + ); + } + + async open(sessionName: string | undefined, url: string, readerMode = false, selector?: string): Promise { + const label = sessionName ?? 'anonymous'; const browser = await chromium.launch({ headless: true, executablePath: process.env.PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH }); try { const context = await browser.newContext(); - - // Restore cookies - await context.addCookies(cookies); - + await this.setupSession(context, sessionName); const page = await context.newPage(); - // Restore localStorage before the page makes any authenticated requests - // by injecting it via init script - await context.addInitScript((entries: Record) => { - for (const [k, v] of Object.entries(entries)) { - window.localStorage.setItem(k, v); - } - }, localStorageData); - - this.logger.log(`[${sessionName}] Opening ${url}`); + this.logger.log(`[${label}] Opening ${url}`); await page.goto(url, { waitUntil: 'networkidle' }); const finalUrl = page.url(); @@ -69,7 +78,13 @@ export class BrowserService { const rawHtml = await page.content(); let content: string; - if (readerMode) { + if (selector) { + const dom = new JSDOM(rawHtml, { url: finalUrl }); + const el = dom.window.document.querySelector(selector); + content = readerMode + ? (el?.textContent?.replace(/\s+/g, ' ').trim() ?? '') + : (el?.outerHTML ?? ''); + } else if (readerMode) { const dom = new JSDOM(rawHtml, { url: finalUrl }); const article = new Readability(dom.window.document).parse(); content = article ? article.textContent.replace(/\s+/g, ' ').trim() : rawHtml; @@ -77,76 +92,35 @@ export class BrowserService { content = rawHtml; } - this.logger.log(`[${sessionName}] Loaded: ${finalUrl} — "${title}"${readerMode ? ' (reader mode)' : ''}`); + this.logger.log(`[${label}] Loaded: ${finalUrl} — "${title}"${readerMode ? ' (reader mode)' : ''}${selector ? ` (selector: ${selector})` : ''}`); return { url: finalUrl, title, content }; } catch (err) { - if ( - err instanceof BadRequestException || - err instanceof NotFoundException || - err instanceof InternalServerErrorException - ) { - throw err; - } - this.logger.error(`[${sessionName}] Open failed: ${(err as Error).message}`); - throw new InternalServerErrorException( - `Browser open failed: ${(err as Error).message}`, - { cause: err }, - ); + this.rethrow(err, label, 'open'); } finally { await browser.close(); } } - async exec(sessionName: string, code: string, url?: string): Promise { - const session = await this.sessionService.findBySessionName(sessionName); - if (!session) { - throw new NotFoundException(`Session not found: ${sessionName}`); - } - - let cookies: any[]; - let localStorageData: Record; - try { - cookies = JSON.parse(session.cookies); - localStorageData = JSON.parse(session.localStorage); - } catch (err) { - throw new InternalServerErrorException('Failed to deserialize session data', { cause: err }); - } - + async exec(sessionName: string | undefined, code: string, url?: string): Promise { + const label = sessionName ?? 'anonymous'; const browser = await chromium.launch({ headless: true, executablePath: process.env.PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH }); try { const context = await browser.newContext(); - await context.addCookies(cookies); - + await this.setupSession(context, sessionName); const page = await context.newPage(); - await context.addInitScript((entries: Record) => { - for (const [k, v] of Object.entries(entries)) { - window.localStorage.setItem(k, v); - } - }, localStorageData); if (url) { - this.logger.log(`[${sessionName}] exec: navigating to ${url}`); + this.logger.log(`[${label}] exec: navigating to ${url}`); await page.goto(url, { waitUntil: 'networkidle' }); } - this.logger.log(`[${sessionName}] exec: running user code`); + this.logger.log(`[${label}] exec: running user code`); const result = await this.codeExecutor.execute(page, context, code); - this.logger.log(`[${sessionName}] exec: done`); + this.logger.log(`[${label}] exec: done`); return result; } catch (err) { - if ( - err instanceof BadRequestException || - err instanceof NotFoundException || - err instanceof InternalServerErrorException - ) { - throw err; - } - this.logger.error(`[${sessionName}] exec failed: ${(err as Error).message}`); - throw new InternalServerErrorException( - `Browser exec failed: ${(err as Error).message}`, - { cause: err }, - ); + this.rethrow(err, label, 'exec'); } finally { await browser.close(); } diff --git a/src/browser/dto/exec.dto.ts b/src/browser/dto/exec.dto.ts index ac1393f..36355c0 100644 --- a/src/browser/dto/exec.dto.ts +++ b/src/browser/dto/exec.dto.ts @@ -2,12 +2,13 @@ import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; import { IsOptional, IsString, IsUrl } from 'class-validator'; export class ExecDto { - @ApiProperty({ - description: 'Session name previously created by POST /login', + @ApiPropertyOptional({ + description: 'Session name previously created by POST /login. If omitted, executes without a stored session.', example: 'test-session-1', }) + @IsOptional() @IsString() - sessionName: string; + sessionName?: string; @ApiPropertyOptional({ description: 'URL to navigate to before executing code. Skipped if omitted.', diff --git a/src/browser/dto/open.dto.ts b/src/browser/dto/open.dto.ts index eb9bb87..3a013da 100644 --- a/src/browser/dto/open.dto.ts +++ b/src/browser/dto/open.dto.ts @@ -2,12 +2,13 @@ import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; import { IsBoolean, IsOptional, IsString, IsUrl } from 'class-validator'; export class OpenDto { - @ApiProperty({ - description: 'Session name previously created by POST /login', + @ApiPropertyOptional({ + description: 'Session name previously created by POST /login. If omitted, opens the URL without a stored session.', example: 'test-session-1', }) + @IsOptional() @IsString() - sessionName: string; + sessionName?: string; @ApiProperty({ description: 'URL to open with the authenticated session', @@ -23,4 +24,12 @@ export class OpenDto { @IsOptional() @IsBoolean() readerMode?: boolean; + + @ApiPropertyOptional({ + description: 'CSS selector whose matching element content is returned. When omitted the full page HTML is used.', + example: '#main-content', + }) + @IsOptional() + @IsString() + selector?: string; } diff --git a/src/mcp/mcp.service.ts b/src/mcp/mcp.service.ts index f429303..9f47cca 100644 --- a/src/mcp/mcp.service.ts +++ b/src/mcp/mcp.service.ts @@ -189,14 +189,15 @@ export class McpService { { description: 'Open a URL using a stored session and return the page title and content', inputSchema: { - sessionName: z.string().describe('Session name to restore cookies and localStorage from'), + sessionName: z.string().optional().describe('Session name to restore cookies and localStorage from. Omit to open without a stored session.'), url: z.string().url().describe('URL to navigate to'), readerMode: z.boolean().optional().describe('Extract readable plain text instead of raw HTML'), + selector: z.string().optional().describe('CSS selector whose matching element content is returned; applied before readerMode'), }, }, - async ({ sessionName, url, readerMode }) => { + async ({ sessionName, url, readerMode, selector }) => { try { - const result = await this.browserService.open(sessionName, url, readerMode ?? false); + const result = await this.browserService.open(sessionName, url, readerMode ?? false, selector); return { content: [{ type: 'text' as const, text: JSON.stringify(result) }] }; } catch (err) { return { isError: true, content: [{ type: 'text' as const, text: (err as Error).message }] }; @@ -209,7 +210,7 @@ export class McpService { { description: 'Execute arbitrary Playwright JavaScript with `page` and `context` in scope', inputSchema: { - sessionName: z.string().describe('Session name to restore'), + sessionName: z.string().optional().describe('Session name to restore. Omit to run without a stored session.'), code: z.string().describe('JavaScript code body to execute (async-safe, may use `page` and `context`)'), url: z.string().url().optional().describe('Optional URL to navigate to before running code'), }, diff --git a/test/__mocks__/jsdom.ts b/test/__mocks__/jsdom.ts index 7e228de..4e81fd6 100644 --- a/test/__mocks__/jsdom.ts +++ b/test/__mocks__/jsdom.ts @@ -1,6 +1,15 @@ +const mockElement = { + outerHTML: '
mock content
', + textContent: 'mock content', +}; + export class JSDOM { constructor(public html: string, public options?: any) {} get window() { - return { document: {} }; + return { + document: { + querySelector: (_selector: string) => mockElement, + }, + }; } } diff --git a/test/browser.controller.spec.ts b/test/browser.controller.spec.ts index 8ecda10..dad3c41 100644 --- a/test/browser.controller.spec.ts +++ b/test/browser.controller.spec.ts @@ -45,13 +45,6 @@ describe('BrowserController', () => { await request(app.getHttpServer()).post('/open').send({}).expect(400); }); - it('returns 400 when sessionName is missing', async () => { - await request(app.getHttpServer()) - .post('/open') - .send({ url: 'https://example.com' }) - .expect(400); - }); - it('returns 400 when url is missing', async () => { await request(app.getHttpServer()) .post('/open') @@ -65,6 +58,34 @@ describe('BrowserController', () => { .send({ sessionName: 'no-such-session', url: 'https://example.com' }) .expect(404); }); + + it('succeeds without a session (sessionless open)', async () => { + const res = await request(app.getHttpServer()) + .post('/open') + .send({ url: 'https://example.com' }) + .expect(201); + expect(res.body).toMatchObject({ + url: expect.any(String), + title: expect.any(String), + content: expect.any(String), + }); + }); + + it('returns only selector content when selector is provided', async () => { + const res = await request(app.getHttpServer()) + .post('/open') + .send({ url: 'https://example.com', selector: '#mock' }) + .expect(201); + expect(res.body.content).toBe('
mock content
'); + }); + + it('returns selector text content in reader mode', async () => { + const res = await request(app.getHttpServer()) + .post('/open') + .send({ url: 'https://example.com', selector: '#mock', readerMode: true }) + .expect(201); + expect(res.body.content).toBe('mock content'); + }); }); // ── POST /exec ───────────────────────────────────────────────────────────── @@ -94,5 +115,13 @@ describe('BrowserController', () => { .send({ sessionName: 'no-such-session', code: 'return 1;' }) .expect(404); }); + + it('succeeds without a session (sessionless exec)', async () => { + const res = await request(app.getHttpServer()) + .post('/exec') + .send({ code: 'return 42;' }) + .expect(201); + expect(res.body).toEqual({ result: 42 }); + }); }); });