Conversation
sroussey
commented
Feb 21, 2026
sroussey
commented
- Launched the initial version (0.0.1) of the @workglow/browser package, providing browser automation capabilities using accessibility trees for stable element selection.
- Implemented features including multi-backend support (Playwright, Electron), session isolation, cookie management, and a task-based architecture for composable browser actions.
- Added comprehensive documentation, examples, and tests to ensure functionality and ease of use.
- Included a changelog and implementation details for better tracking of changes and features.
- Launched the initial version (0.0.1) of the @workglow/browser package, providing browser automation capabilities using accessibility trees for stable element selection. - Implemented features including multi-backend support (Playwright, Electron), session isolation, cookie management, and a task-based architecture for composable browser actions. - Added comprehensive documentation, examples, and tests to ensure functionality and ease of use. - Included a changelog and implementation details for better tracking of changes and features.
There was a problem hiding this comment.
Pull request overview
This PR introduces the initial version (0.0.1) of the @workglow/browser package, providing comprehensive browser automation capabilities using accessibility trees for stable element selection. The package supports multiple backends (Playwright, Electron, and remote browser services like Browserless, Browserbase, and Bright Data), includes session isolation features, cookie management, and a chainable task-based API for composable browser actions.
Changes:
- New browser automation package with 6 backend implementations
- Accessibility tree-based element selection system
- Cookie store with domain/path scoping and serialization
- 8 browser automation tasks (navigate, click, type, extract, wait, screenshot, run script, browser init)
Reviewed changes
Copilot reviewed 56 out of 57 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/browser/package.json | Package configuration with multi-target builds and peer dependencies |
| packages/browser/tsconfig.json | TypeScript configuration for the browser package |
| packages/browser/src/a11y/* | Accessibility tree parser and query system |
| packages/browser/src/context/* | Browser context implementations for Playwright, Electron, and remote services |
| packages/browser/src/task/* | Browser automation task implementations |
| packages/browser/src/workflow/* | Workflow integration and helper functions |
| packages/browser/src/test/* | Comprehensive test suite with 40+ passing tests |
| packages/browser/docs/* | Documentation for session isolation, partitions, and remote browsers |
| packages/browser/examples/* | Working examples demonstrating usage patterns |
| test-electron/* | Electron integration test application |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const label = document.querySelector(`label[for="${htmlElement.id}"]`); | ||
| if (label) { | ||
| const labelText = label.textContent?.trim(); | ||
| if (labelText) { | ||
| return labelText; |
There was a problem hiding this comment.
The computeAccessibleName function uses document.getElementById() with an unsanitized id from aria-labelledby attribute. While this is generally safe because getElementById does not execute code, the selector construction on line 513 uses string interpolation with htmlElement.id directly in querySelector(). This could be vulnerable to selector injection if the ID contains special characters. Consider using document.getElementById(htmlElement.id) instead or sanitizing the ID before constructing the selector.
| const label = document.querySelector(`label[for="${htmlElement.id}"]`); | |
| if (label) { | |
| const labelText = label.textContent?.trim(); | |
| if (labelText) { | |
| return labelText; | |
| const labels = document.getElementsByTagName("label"); | |
| for (let i = 0; i < labels.length; i++) { | |
| const labelElement = labels[i] as HTMLLabelElement; | |
| if (labelElement.htmlFor === htmlElement.id) { | |
| const labelText = labelElement.textContent?.trim(); | |
| if (labelText) { | |
| return labelText; | |
| } | |
| break; |
| // Close with a timeout to prevent hanging | ||
| const closePromise = (async () => { | ||
| try { | ||
| if (this.context) { | ||
| await this.context.close(); | ||
| } | ||
| } catch { | ||
| // Ignore errors during close | ||
| } | ||
| })(); | ||
|
|
||
| // Race against a 5-second timeout | ||
| await Promise.race([ | ||
| closePromise, | ||
| new Promise<void>((resolve) => setTimeout(resolve, 5000)), | ||
| ]); | ||
|
|
||
| // Always clear references | ||
| this.page = undefined; | ||
| this.context = undefined; | ||
| this.browser = undefined; | ||
| this.responseHandler = undefined; | ||
|
|
||
| // Remove from global tracker | ||
| activeContexts.delete(this); | ||
|
|
||
| // Small delay to let browser process fully terminate | ||
| await new Promise((resolve) => setTimeout(resolve, 100)); | ||
| } |
There was a problem hiding this comment.
The close() method races the cleanup against a 5-second timeout, but then always clears references regardless of whether context.close() completed successfully. If the close promise is still running after the timeout, this could result in attempting to close an already-freed context or leave resources hanging. Consider awaiting the close promise with a timeout wrapper instead of racing, or add a flag to track whether cleanup completed.
|
@copilot open a new pull request to apply changes based on the comments in this thread |
|
@copilot fix issues found via build/test |
