Skip to content

feat(tools): add a browser tool - #1626

Merged
addyCooks merged 9 commits into
Nano-Collective:mainfrom
Dhirenderchoudhary:feat/1488-browser
Oct 9, 2026
Merged

addyCooks merged 9 commits into
Nano-Collective:mainfrom
Dhirenderchoudhary:feat/1488-browser

Conversation

@Dhirenderchoudhary

@Dhirenderchoudhary Dhirenderchoudhary commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Description

Adds a built-in browser tool so the agent can open a page, click, type, and see a screenshot. Screenshot bytes travel as image parts on the tool result, including when a subagent runs the tool. Fixes #1488.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

Changeset

  • Added a changeset (pnpm changeset) describing this change for the changelog

Docs-only or internal chores need no changeset (or run pnpm changeset --empty to note that intentionally).

Testing

Automated Tests

  • New features include passing tests in .spec.ts/tsx files
  • All existing tests pass (pnpm test:all completes successfully)
  • Tests cover both success and error scenarios

Manual Testing

  • Tested with Ollama
  • Tested with OpenRouter
  • Tested with OpenAI-compatible API
  • Tested MCP integration (if applicable)

Checklist

  • If this was for an open issue, I was assigned to it
  • Code follows project style guidelines
  • Self-review completed
  • Documentation updated (if needed)
  • No breaking changes (or clearly documented)
  • Appropriate logging added using structured logging (see CONTRIBUTING.md)

@github-actions github-actions Bot added area:tools Tool implementations and tool-calling area:tui Terminal UI area:docs Documentation labels Oct 7, 2026
@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

nc-review: needs work — 1 blocking, 3 important, 1 nit

@Dhirenderchoudhary — there is a blocking item below.

Adds a Playwright-backed browser tool (navigate/click/type/screenshot) with a new VisualToolOutput type that carries images on a tool result. The shape and shape-routing of those images through convertToModelMessages (output {type: 'content', value: [...]} containing {type: 'image-data', data, mediaType}) is not a tool-result output type used anywhere else in this codebase — the only tool-result output types in flight are text and json, and the only image part type used today is {type: 'image', image: dataUrl} for user content — so the vision-model integration the PR promises is unverified against the AI SDK and providers, and the screenshot bytes may be silently dropped at the provider boundary.

🔴 blocking · correctness · source/ai-sdk-client/converters/message-converter.ts

The new image branch emits a tool-result with output: {type: 'content', value: [...]} whose parts are {type: 'text', text} and {type: 'image-data', data, mediaType}. Neither shape is a documented AI SDK tool-result output/part type in this codebase:

  • The only tool-result output.type values the rest of the code produces or tests are 'text' and 'json' (see message-converter.ts:159-178, preparestep.spec.ts:89/100/129, message-converter.spec.ts).
  • The only image-part type used today is {type: 'image', image: dataUrl, mediaType} for user messages (message-converter.ts:214). No call site uses {type: 'image-data', data, mediaType}.
  • The type: 'content' token that appears in source/acp/acp-tool-call.ts:168 and source/vscode/chat-panel-tool-cards.spec.ts:229 is an ACP / VS Code card shape, not an AI SDK tool-result output type.

The new unit test (message-converter.spec.ts, "emits image-data parts for tool screenshots") only asserts the converted structure via as casts — it never exercises a provider. If the AI SDK doesn't accept output.type === 'content' for a tool result, the image parts will be silently dropped or cause a provider 400, breaking the headline claim that "a vision model can see the page". Verify by running a screenshot through a vision-capable provider end-to-end before merging; if 'content' is not valid, fall back to one of the documented shapes (e.g. an AI SDK image part inside a content array) and update the test to mirror what a real provider actually accepts.

🟠 important · design · source/message-handler.ts

The early return for VisualToolOutput (if (result && typeof result === 'object' && 'images' in result) { return ... }) bypasses the structured-output pipeline: formatWrittenFile, appendPostToolUseOutput (post-tool-use hooks), and maybeAutoCommit. The structured path that returns next in the file still runs all of those. That asymmetry is intentional for browser (none of those apply to a navigate/click/type), but it means a tool that ever wants both images and post-tool-use hooks has to choose: either carry the hooks itself or stop using VisualToolOutput. A short comment on the new branch noting that the bypass is deliberate (and which of the three skips apply to browser) would prevent the next reader from "fixing" it by re-routing through the structured branch and silently dropping the images.

🟠 important · correctness · source/types/core.ts

VisualToolOutput has no isError field, while StructuredToolOutput does. The early return in processToolUse drops isError propagation for visual results (the branch never sets isError: true and VisualToolOutput can't carry it). The browser tool will throw on bad selectors / dead servers, which the catch block handles — so this isn't currently observable — but it does mean the JSON/ACP telemetry that distinguishes failed tool calls from successful ones cannot see a visual tool that fails without throwing. Either add isError?: boolean to VisualToolOutput and propagate it in the early return, or call this out as a deliberate gap in the type comment.

🟠 important · tests · source/ai-sdk-client/converters/message-converter.spec.ts

The new "emits image-data parts for tool screenshots" test asserts the converter output shape but does not exercise a provider. Combined with the unverifiable type: 'content' / type: 'image-data' shapes in the finding above, the test passes while proving nothing about whether a vision-capable model actually receives the screenshot. Either drop the test down to a verified shape (after confirming one) or pair it with an integration test that hits a vision-capable provider with a real screenshot tool result.

⚪ nit · scope · source/subagents/subagent-executor.ts

I could not fully reconcile the diff hunk @@ -613,21 +614,29 @@ against the on-disk file. The on-disk executeToolCall body still calls formatWrittenFile, appendPostToolUseOutput, and maybeAutoCommit; the lines shown in the diff don't include any of them, so it's not clear whether the PR removes those calls for all subagent tool calls (which would be a silent regression for post-tool-use hooks across every tool, not just browser) or only the per-call return shape is changing. Worth a maintainer eyeball to confirm the merge keeps the formatter/hooks/auto-commit path intact for all delegated tool calls.


🔴 blocking · 🟠 a reviewer would ask for a change · ⚪ optional

Automated code review — correctness, security, design, tests, plus duplicates and scope. A human still decides; this is not a substitute for review and is not exhaustive. The required status checks separately cover lint, formatting, types, unused dependencies, the test suite and the build. This bot never merges. Maintainers can rerun with /re-review.

@github-actions github-actions Bot added the agent:needs-work nc-review found blocking findings label Oct 7, 2026
@addyCooks

Copy link
Copy Markdown
Collaborator

Thanks @Dhirenderchoudhary, this is a big step and the structure is clean (lazy Playwright
import, shared page with shutdown hook, plan-mode exclusion, VisualToolOutput carrying
isError, and the subagent path still runs formatters/hooks/auto-commit).
I ran types, lint, format, knip and the related specs locally and they pass. A few things before merge:

  1. Screenshots don't reach the model on openai-compatible providers (the default for Ollama,
    LM Studio, most OpenRouter). @ai-sdk/openai-compatible JSON.stringify()s a tool result of
    output.type === 'content', so the base64 image goes into the tool message as ~80 KB of
    text and no image part is sent. I confirmed this by running convertToModelMessages through
    createOpenAICompatible with a mock fetch and capturing the request body. Anthropic, OpenAI
    and Google map image-data correctly. Could we handle openai-compatible separately (for
    example send the image as an image_url content part), and add a test that checks the
    request body for that provider?
  2. In headless mode the browser tool needs no approval and has no host guard, unlike
    fetch_url (fetch-url-guard.ts blocks loopback/private/169.254.x). Could we apply the same
    guard there, or at least require approval for non-localhost targets?
  3. The tool is offered to every model. Could it only be exposed when the model accepts image
    input (models.dev modalities), or fall back to text-only output?

Smaller: browser-page.ts caches the page forever, so a crashed browser stays broken until
restart; resetting opening on a closed-page error would fix that.

Happy to help with any of these.

@Dhirenderchoudhary

Copy link
Copy Markdown
Contributor Author

Thanks @addyCooks, pushed fixes for all four:

  1. On openai-compatible, the screenshot now goes in a user message right after the tool result, since Chat Completions doesn't allow images in tool messages. Added a test that checks the actual request body.
  2. Private and metadata IPs are blocked now, same check as fetch_url, but localhost still works. Redirects into them get caught too.
  3. If models.dev says the model is text-only, it gets the caption without the image.
  4. A crashed browser gets relaunched on the next call.

@addyCooks addyCooks left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks @Dhirenderchoudhary, nice turnaround.
I re-checked the new head (4f149d1) and all four points are addressed:

  1. openai-compatible screenshots: the tool message is now text-only and the image arrives as an image_url part in a following user message. I re-ran my request-body probe against the real @ai-sdk/openai-compatible provider: the tool message is ~30 chars with no base64, and the order is valid (user > assistant > tool > user). Anthropic, Google and OpenAI Responses keep the inline path.
  2. Host guard: I probed 18 URLs. Metadata, 10.x / 172.16.x / 192.168.x / 100.64.x, 0.0.0.0, IPv6 private and mapped forms, and hex/decimal IP encodings are blocked; localhost, 127.0.0.1, ::1 and *.localhost still work. Redirect landings are caught too.
  3. Models listed as text-only get the caption without the image.
  4. A crashed or disconnected browser relaunches on the next call.

Types, lint, format, knip and the related specs pass locally, and CI is green.

Known limits, none blocking (happy to track them as follow-ups):

  • The guard checks literal hostnames only, so a public name that resolves to a private IP is not blocked. fetch_url has the same limit today, and #1662 is working on it.
  • Models that models.dev doesn't list (most local ones) still receive the image, so a text-only local model may reject it.
  • Localhost stays reachable in headless mode by design.

Approving 🎉

@addyCooks
addyCooks merged commit 11c275b into Nano-Collective:main Oct 9, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent:needs-work nc-review found blocking findings area:docs Documentation area:tools Tool implementations and tool-calling area:tui Terminal UI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] First-class Computer-Use / Browser Automation Tool

2 participants