Skip to content

feat: promote eval to top-level command, add --frame for iframes#200

Open
sfc-gh-mochen wants to merge 8 commits intoszymdzum:mainfrom
Snowflake-Labs:mochen-eval-and-frame
Open

feat: promote eval to top-level command, add --frame for iframes#200
sfc-gh-mochen wants to merge 8 commits intoszymdzum:mainfrom
Snowflake-Labs:mochen-eval-and-frame

Conversation

@sfc-gh-mochen
Copy link
Copy Markdown
Contributor

Summary

  • Promote bdg dom eval to bdg eval as a top-level command
  • Add FrameOptions interface and --frame flag for DOM commands to target iframes
  • Add error logging utility (src/utils/errorLog.ts)
  • Update all references, formatters, error messages, and tests

Test plan

  • Build passes (npm run build)
  • Run bdg eval "document.title" to verify top-level eval works
  • Run bdg dom query "selector" --frame "iframe" to verify iframe targeting
  • Run existing test suite

sfc-gh-mochen and others added 8 commits December 23, 2025 18:20
- Remove session.json file write on stop
- Remove offline session fallback from network commands
- Delete unused output.ts module
- Update skill to discourage premature stop calls
- Remove 'save output' references from UI messages
- Demote 'bdg stop' in command listings (show peek/tail first)
- Update golden workflow test to use peek instead of session.json
- Users should use bdg peek to inspect telemetry data
# Conflicts:
#	package-lock.json
Add Playwright-style pseudo-selectors:
- :has-text("text") - element contains text (case-insensitive)
- :text("text") - smallest element with exact text
- :text-is("text") - exact text match (case-sensitive)
- :visible - element is visible

Remove nodeId caching layer:
- Delete DomElementResolver and QueryCacheManager
- Remove index-based operations (bdg dom click 0)
- Always use selectors with optional --index flag

Bug fixes:
- Fix "Object reference chain is too long" in bdg dom eval by handling
  non-serializable DOM elements gracefully
- Fix script execution errors by embedding QUERY_ELEMENTS_HELPER inside
  each IIFE instead of concatenating before it
- Update "Next steps" to show --index when multiple matches found
- Add Playwright Selectors section with :has-text, :text, :text-is, :visible
- Explain why Playwright selectors replace 2-step query+index workflow
- Update Form Interaction to show selector-based approach with --index
- Document that dom eval now handles DOM elements gracefully
- Remove references to index-based operations (bdg dom fill 0)
- Add 'Click Button by Text' pattern example
- Add --chrome-ws-url option to screenshot command for direct CDP
  connection without daemon
- Add top-level `bdg screenshot` alias for `dom screenshot`
- Create directMode module for one-shot CDP commands
* fix: remove --chrome-ws-url from root command to fix subcommand option conflict

The --chrome-ws-url option was defined on both the root program (via
applyCollectorOptions in registerStartCommands) and on the dom screenshot
subcommand. This caused Commander.js to parse the option on the root
command instead of passing it to the subcommand handler.

The fix removes --chrome-ws-url from applyCollectorOptions since:
1. The start command (root) is for starting new sessions with URLs
2. Direct mode (--chrome-ws-url) is only relevant for dom/cdp commands
   that connect to existing Chrome instances

This fixes: bdg dom screenshot --chrome-ws-url <url> now works correctly.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* feat: add `bdg electron` command for Electron app automation

Add support for connecting to running Electron apps via CDP:

- `bdg electron` - Connect to Electron on default port 9229
- `bdg electron --list` - List available Electron targets
- `bdg electron --port <port>` - Connect to specific port
- `bdg electron --target <id>` - Connect to specific target

After connecting, all DOM commands work normally (screenshot, query,
eval, click, etc.) through the daemon session.

New files:
- src/connection/electronDiscovery.ts - Target discovery functions
- src/commands/electron.ts - The electron command

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
…e targeting

- Move `bdg dom eval` to `bdg eval` as a top-level command
- Add FrameOptions interface and --frame flag for DOM commands to target iframes
- Add error logging utility
- Update all references, formatters, messages, and tests
@szymdzum
Copy link
Copy Markdown
Owner

Hey @sfc-gh-mochen, thanks for the substantial work here. I've been catching up on the backlog and reviewed all three related PRs (#169, #199, #200).

The features are solid, but these PRs have a lot of overlap and bundled changes that make them hard to merge safely. Here's where things stand:

Already merged:

The ask:
Could you rebase #200 onto current main and split it into focused PRs? Ideally:

  1. DOM refactor PR - Playwright selectors + nodeId caching removal (the core of feat(dom): add Playwright selector support, remove nodeId caching #169). This is the foundation everything else depends on.
  2. Eval promotion PR - bdg eval as top-level + --frame for iframes
  3. Electron/direct mode PR - bdg electron + --chrome-ws-url bypass

Issues to address in the DOM refactor:

  • The __bdgQueryElements JS helper is duplicated in helpers.ts and reactEventHelpers.ts with subtle differences (whitespace normalization: \\s+ vs +). Should be a single shared source.
  • src/selectors/playwrightSelectors.ts (325 lines) + css-selector-parser dependency are added but never imported at runtime. Either use it or remove it.
  • executeScript in evalHelpers.ts runs expressions twice (once with returnByValue: false, then true), causing side effects to fire twice.
  • Empty catch block in evalHelpers.ts.
  • Dead error message functions left behind (elementAtIndexNotFoundError, indexOutOfRangeError).

For electron.ts specifically:

  • Uses console.error + process.exit instead of CommandRunner + CommandError pattern.
  • errorLog.ts writes to hardcoded /tmp with no rotation.

Re: #169 - since #200 is a superset, I'd suggest closing #169 once #200 is split and the DOM refactor PR is ready.

Re: #199 (@sfc-gh-adsaxena's WebSocket capture) - the websocket feature itself is great, but it also bundles the same DOM refactor. Once your DOM refactor PR lands, the websocket PR can be rebased cleanly onto it. I'll comment on #199 separately.

Breaking changes are fine. Happy to help review the split PRs. Thanks for contributing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants