← Back to PRs

#3794: fix(browser-tool): disallow close without targetId to avoid unsafe tab ownership

by JaydenLiang open 2026-01-29 05:53 View on GitHub →
agents
## PR Description ### Background The browser tool previously allowed the close action to fall back to closing the active tab when no `targetId` was provided. This implicit behavior relies on global browser state and does not establish clear ownership of the tab being closed. While convenient, this pattern is unsafe in both concurrent automation scenarios and human-driven Chrome relay usage. --- ### Problem Without an explicit `targetId`: - Tab ownership cannot be reliably inferred. - In concurrent runs, one task may accidentally close a tab created or used by another task. - In Chrome relay (human-driven) scenarios, the tool may close a user-owned tab that was never explicitly authorized for closure. This makes `close` a non-deterministic and potentially destructive action. --- ### Changes - Enforce an explicit `targetId` for the `close` action. - Remove the implicit fallback to closing the active tab. - Fail fast with a clear error message when `close` is invoked without a `targetId`. This makes tab closure deterministic and ownership-safe across all browser profiles. --- ### Impact - **Breaking change (intentional):** callers must now track and supply `targetId` when closing tabs. - Developers and users need to maintain the tabs related to their uses. - Improves safety and predictability in concurrent agent runs. - Prevents accidental tab closure in Chrome relay / human-driven scenarios. - Aligns destructive browser actions with explicit ownership semantics. --- ### Unit test Test cases for the changes are included. --- ### Notes / Follow-ups - Other browser actions remain unchanged. - This PR establishes a precedent that destructive actions must be explicit and ownership-aware. - Documentation and examples may be updated in follow-up PRs if needed. <!-- greptile_comment --> <h2>Greptile Overview</h2> <h3>Greptile Summary</h3> This PR aims to make the browser tool’s `close` action deterministic by requiring an explicit `targetId` instead of implicitly closing the active tab. The main implementation change is in `src/agents/tools/browser-tool.ts`, where the non-proxy path now throws an error if `targetId` is missing, and new unit tests were added in `src/agents/tools/browser-tool.test.ts`. One critical gap remains: when routing through the node browser proxy, `close` still falls back to `POST /act` without a `targetId`, which preserves the unsafe behavior for proxy usage and conflicts with the PR’s stated breaking change. <h3>Confidence Score: 2/5</h3> - This PR is not safe to merge as-is because the node-proxy code path still allows closing without a targetId. - While the non-proxy path now fails fast without `targetId`, the `proxyRequest` branch still performs an implicit close via `/act` when `targetId` is omitted, undermining the safety goal. Additionally, the new chrome-profile unit test is currently misconfigured and doesn’t validate the intended behavior. - src/agents/tools/browser-tool.ts, src/agents/tools/browser-tool.test.ts <!-- greptile_other_comments_section --> <sub>(4/5) You can add custom instructions or style guidelines for the agent [here](https://app.greptile.com/review/github)!</sub> <!-- /greptile_comment -->

Most Similar PRs