claudetool: streamline browser tools
browser_click, browser_type, browser_get_text, browser_scroll_into_view,
browser_resize, and wait_for can all be easily implemented using browser_eval,
if browser_eval is given an await parameter.
A bit of testing suggests that they are more robust in practice
that way, and that multiple tool calls can be combined
into a single browser_eval call, which reduces latency.
And Sketch does in fact use them as needed.
Also, bump up timeouts; empirically, 5 seconds is not enough.
As a bonus, fewer tools is better for context management.
Co-Authored-By: sketch <hello@sketch.dev>
Change-ID: s8d8cd418f3e97f26k
diff --git a/claudetool/browse/browse_test.go b/claudetool/browse/browse_test.go
index d1b1eec..def464b 100644
--- a/claudetool/browse/browse_test.go
+++ b/claudetool/browse/browse_test.go
@@ -30,13 +30,8 @@
requiredProps []string
}{
{tools.NewNavigateTool(), "browser_navigate", "Navigate", []string{"url"}},
- {tools.NewClickTool(), "browser_click", "Click", []string{"selector"}},
- {tools.NewTypeTool(), "browser_type", "Type", []string{"selector", "text"}},
- {tools.NewWaitForTool(), "browser_wait_for", "Wait", []string{"selector"}},
- {tools.NewGetTextTool(), "browser_get_text", "Get", []string{"selector"}},
{tools.NewEvalTool(), "browser_eval", "Evaluate", []string{"expression"}},
{tools.NewScreenshotTool(), "browser_take_screenshot", "Take", nil},
- {tools.NewScrollIntoViewTool(), "browser_scroll_into_view", "Scroll", []string{"selector"}},
}
for _, tt := range toolTests {
@@ -78,8 +73,8 @@
// Test with screenshot tools included
t.Run("with screenshots", func(t *testing.T) {
toolsWithScreenshots := tools.GetTools(true)
- if len(toolsWithScreenshots) != 12 {
- t.Errorf("expected 12 tools with screenshots, got %d", len(toolsWithScreenshots))
+ if len(toolsWithScreenshots) != 6 {
+ t.Errorf("expected 6 tools with screenshots, got %d", len(toolsWithScreenshots))
}
// Check tool naming convention
@@ -94,8 +89,8 @@
// Test without screenshot tools
t.Run("without screenshots", func(t *testing.T) {
noScreenshotTools := tools.GetTools(false)
- if len(noScreenshotTools) != 10 {
- t.Errorf("expected 10 tools without screenshots, got %d", len(noScreenshotTools))
+ if len(noScreenshotTools) != 4 {
+ t.Errorf("expected 4 tools without screenshots, got %d", len(noScreenshotTools))
}
})
}
@@ -382,61 +377,3 @@
t.Errorf("Expected default height %v, got %v", expectedHeight, response.Height)
}
}
-
-// TestResizeTool tests the browser resize functionality
-func TestResizeTool(t *testing.T) {
- ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
- defer cancel()
-
- // Skip if CI or headless testing environment
- if os.Getenv("CI") != "" || os.Getenv("HEADLESS_TEST") != "" {
- t.Skip("Skipping browser test in CI/headless environment")
- }
-
- t.Run("ResizeWindow", func(t *testing.T) {
- tools := NewBrowseTools(ctx)
- t.Cleanup(func() {
- tools.Close()
- })
-
- // Resize to mobile dimensions
- resizeTool := tools.NewResizeTool()
- input := json.RawMessage(`{"width": 375, "height": 667}`)
- toolOut := resizeTool.Run(ctx, input)
- if toolOut.Error != nil {
- t.Fatalf("Error: %v", toolOut.Error)
- }
- content := toolOut.LLMContent
- if !strings.Contains(content[0].Text, "done") {
- t.Fatalf("Expected done in response, got: %s", content[0].Text)
- }
-
- // Navigate to a test page and verify using JavaScript to get window dimensions
- navInput := json.RawMessage(`{"url": "https://example.com"}`)
- toolOut = tools.NewNavigateTool().Run(ctx, navInput)
- if toolOut.Error != nil {
- t.Fatalf("Error: %v", toolOut.Error)
- }
- content = toolOut.LLMContent
- if !strings.Contains(content[0].Text, "done") {
- t.Fatalf("Expected done in response, got: %s", content[0].Text)
- }
-
- // Check dimensions via JavaScript
- evalInput := json.RawMessage(`{"expression": "({width: window.innerWidth, height: window.innerHeight})"}`)
- toolOut = tools.NewEvalTool().Run(ctx, evalInput)
- if toolOut.Error != nil {
- t.Fatalf("Error: %v", toolOut.Error)
- }
- content = toolOut.LLMContent
-
- // The dimensions might not be exactly what we set (browser chrome, etc.)
- // but they should be close
- if !strings.Contains(content[0].Text, "width") {
- t.Fatalf("Expected width in response, got: %s", content[0].Text)
- }
- if !strings.Contains(content[0].Text, "height") {
- t.Fatalf("Expected height in response, got: %s", content[0].Text)
- }
- })
-}