fix: address code review feedback on browser tool
- Handle ignored error from AddCookies in cookie delete operation - Check os.MkdirAll error in workspace directory creation - Store Pages() result in variable to avoid redundant calls - Document CdpURL/WsURL field precedence in BrowserConfig Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
71420794aa
commit
113d6705d8
3 changed files with 15 additions and 6 deletions
|
|
@ -122,7 +122,10 @@ func createToolRegistry(workspace string, restrict bool, cfg *config.Config, msg
|
|||
|
||||
func NewAgentLoop(cfg *config.Config, msgBus *bus.MessageBus, provider providers.LLMProvider) *AgentLoop {
|
||||
workspace := cfg.WorkspacePath()
|
||||
os.MkdirAll(workspace, 0755)
|
||||
if err := os.MkdirAll(workspace, 0755); err != nil {
|
||||
logger.ErrorCF("agent", "Failed to create workspace directory",
|
||||
map[string]interface{}{"path": workspace, "error": err.Error()})
|
||||
}
|
||||
|
||||
restrict := cfg.Agents.Defaults.RestrictToWorkspace
|
||||
|
||||
|
|
|
|||
|
|
@ -213,8 +213,8 @@ type WebToolsConfig struct {
|
|||
type BrowserConfig struct {
|
||||
Enabled bool `json:"enabled" env:"PICOCLAW_TOOLS_BROWSER_ENABLED"`
|
||||
Protocol string `json:"protocol" env:"PICOCLAW_TOOLS_BROWSER_PROTOCOL"`
|
||||
CdpURL string `json:"cdp_url" env:"PICOCLAW_TOOLS_BROWSER_CDP_URL"`
|
||||
WsURL string `json:"ws_url" env:"PICOCLAW_TOOLS_BROWSER_WS_URL"`
|
||||
CdpURL string `json:"cdp_url" env:"PICOCLAW_TOOLS_BROWSER_CDP_URL"` // WebSocket URL for CDP protocol (Chromium/Browserless)
|
||||
WsURL string `json:"ws_url" env:"PICOCLAW_TOOLS_BROWSER_WS_URL"` // WebSocket URL for Playwright wire protocol (Firefox/Camoufox); takes precedence over CdpURL if set
|
||||
Token string `json:"token" env:"PICOCLAW_TOOLS_BROWSER_TOKEN"`
|
||||
Stealth bool `json:"stealth" env:"PICOCLAW_TOOLS_BROWSER_STEALTH"`
|
||||
LaunchTimeout int `json:"launch_timeout" env:"PICOCLAW_TOOLS_BROWSER_LAUNCH_TIMEOUT"`
|
||||
|
|
|
|||
|
|
@ -240,8 +240,12 @@ func (t *BrowserTool) ensureConnected() error {
|
|||
// Reuse existing page from browser context if available (common for CDP)
|
||||
var page playwright.Page
|
||||
contexts := browser.Contexts()
|
||||
if len(contexts) > 0 && len(contexts[0].Pages()) > 0 {
|
||||
page = contexts[0].Pages()[0]
|
||||
var pages []playwright.Page
|
||||
if len(contexts) > 0 {
|
||||
pages = contexts[0].Pages()
|
||||
}
|
||||
if len(pages) > 0 {
|
||||
page = pages[0]
|
||||
} else {
|
||||
page, err = browser.NewPage()
|
||||
if err != nil {
|
||||
|
|
@ -623,7 +627,9 @@ func (t *BrowserTool) doCookies(args map[string]interface{}) *ToolResult {
|
|||
}
|
||||
|
||||
if len(toReAdd) > 0 {
|
||||
browserCtx.AddCookies(toReAdd)
|
||||
if err := browserCtx.AddCookies(toReAdd); err != nil {
|
||||
return &ToolResult{ForLLM: fmt.Sprintf("Error re-adding cookies after delete: %v", err)}
|
||||
}
|
||||
}
|
||||
|
||||
return &ToolResult{ForLLM: fmt.Sprintf("Cookie %q deleted", name)}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue