Skip to content

fix(browser): change default wait_until from load to domcontentloaded #68

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
OhYee wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(browser): change default wait_until from load to domcontentloaded #68

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 14 additions & 8 deletions agentrun/integration/builtin/sandbox.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -751,20 +751,26 @@ def _get_playwright(self, sb: BrowserSandbox) -> "BrowserPlaywrightSync":
Automatically recreates the connection when the thread that created it has exited,
because Playwright's internal greenlet is bound to the thread that created it.
"""
if self._playwright_sync is not None and self._playwright_thread is not None:
if (
self._playwright_sync is not None
and self._playwright_thread is not None
):
current_thread = threading.current_thread()
creator_thread = self._playwright_thread
if not creator_thread.is_alive() or current_thread is not creator_thread:
if (
not creator_thread.is_alive()
or current_thread is not creator_thread
):
if not creator_thread.is_alive():
logger.debug(
"Playwright creating thread (id=%s) has exited, recreating"
" connection",
"Playwright creating thread (id=%s) has exited,"
" recreating connection",
creator_thread.ident,
)
else:
logger.debug(
"Playwright creating thread (id=%s) differs from current"
" thread (id=%s), recreating connection",
"Playwright creating thread (id=%s) differs from"
" current thread (id=%s), recreating connection",
creator_thread.ident,
current_thread.ident,
)
Expand Down Expand Up @@ -970,7 +976,7 @@ def goto(self, url: str) -> Dict[str, Any]:
)
def browser_navigate_back(
self,
wait_until: str = "load",
wait_until: str = "domcontentloaded",
timeout: Optional[float] = None,
Comment on lines +979 to 980
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR description references fixing issue #66, but the code change here only updates the default wait_until value for back/forward navigation. Please either update the PR description/issue reference to match the actual change, or include the missing code changes intended to address #66.

Copilot uses AI. Check for mistakes.
) -> Dict[str, Any]:
"""返回上一页 / Go back to previous page"""
Expand All @@ -996,7 +1002,7 @@ def inner(sb: Sandbox):
)
def browser_go_forward(
self,
wait_until: str = "load",
wait_until: str = "domcontentloaded",
timeout: Optional[float] = None,
Comment on lines 1004 to 1006
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR changes behavior by altering the default wait_until for browser_navigate_back/browser_go_forward, but there are no unit tests asserting the new defaults. Consider adding a small test (e.g., via inspect.signature) to lock in the new default and prevent accidental regressions.

Copilot uses AI. Check for mistakes.
Copy link
Member Author

@OhYee OhYee Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback

) -> Dict[str, Any]:
"""前进到下一页 / Go forward to next page"""
Expand Down
Loading