diff --git a/cmd/gateway_setup.go b/cmd/gateway_setup.go index 0f2b2ca9..f6e78457 100644 --- a/cmd/gateway_setup.go +++ b/cmd/gateway_setup.go @@ -6,6 +6,7 @@ import ( "log/slog" "os" "path/filepath" + "time" "github.com/google/uuid" @@ -96,6 +97,18 @@ func setupToolRegistry( opts = append(opts, browser.WithHeadless(cfg.Tools.Browser.Headless)) slog.Info("browser tool enabled", "headless", cfg.Tools.Browser.Headless) } + if cfg.Tools.Browser.ActionTimeoutMs > 0 { + opts = append(opts, browser.WithActionTimeout(time.Duration(cfg.Tools.Browser.ActionTimeoutMs)*time.Millisecond)) + } + if cfg.Tools.Browser.IdleTimeoutMs > 0 { + opts = append(opts, browser.WithIdleTimeout(time.Duration(cfg.Tools.Browser.IdleTimeoutMs)*time.Millisecond)) + } else if cfg.Tools.Browser.IdleTimeoutMs < 0 { + // Explicitly disable idle reaper with negative value + opts = append(opts, browser.WithIdleTimeout(0)) + } + if cfg.Tools.Browser.MaxPages > 0 { + opts = append(opts, browser.WithMaxPages(cfg.Tools.Browser.MaxPages)) + } browserMgr = browser.New(opts...) toolsReg.Register(browser.NewBrowserTool(browserMgr)) } diff --git a/internal/config/config_channels.go b/internal/config/config_channels.go index c889c636..f247a848 100644 --- a/internal/config/config_channels.go +++ b/internal/config/config_channels.go @@ -394,9 +394,12 @@ type WebFetchPolicyConfig struct { // BrowserToolConfig controls the browser automation tool. type BrowserToolConfig struct { - Enabled bool `json:"enabled"` // enable the browser tool (default false) - Headless bool `json:"headless,omitempty"` // run Chrome in headless mode (ignored when RemoteURL is set) - RemoteURL string `json:"remote_url,omitempty"` // CDP endpoint for remote Chrome sidecar, e.g. "ws://chrome:9222" + Enabled bool `json:"enabled"` // enable the browser tool (default false) + Headless bool `json:"headless,omitempty"` // run Chrome in headless mode (ignored when RemoteURL is set) + RemoteURL string `json:"remote_url,omitempty"` // CDP endpoint for remote Chrome sidecar, e.g. "ws://chrome:9222" + ActionTimeoutMs int `json:"action_timeout_ms,omitempty"` // per-action timeout in ms (default 30000) + IdleTimeoutMs int `json:"idle_timeout_ms,omitempty"` // idle page auto-close in ms (default 600000, 0=disabled) + MaxPages int `json:"max_pages,omitempty"` // max open pages per tenant (default 5) } // ToolPolicySpec defines a tool policy at any level (global, per-agent, per-provider). diff --git a/pkg/browser/browser.go b/pkg/browser/browser.go index 338c4ec4..c6ee4146 100644 --- a/pkg/browser/browser.go +++ b/pkg/browser/browser.go @@ -5,6 +5,7 @@ import ( "fmt" "log/slog" "sync" + "time" "github.com/go-rod/rod" "github.com/go-rod/rod/lib/launcher" @@ -19,9 +20,14 @@ type Manager struct { console map[string][]ConsoleMessage // targetID → console messages tenantCtxs map[string]*rod.Browser // tenantID → incognito browser context pageTenants map[string]string // targetID → tenantID (for filtering) - headless bool - remoteURL string // CDP endpoint for remote Chrome (sidecar); skips local launcher - logger *slog.Logger + pageLastUsed map[string]time.Time // targetID → last access time + headless bool + remoteURL string // CDP endpoint for remote Chrome (sidecar); skips local launcher + actionTimeout time.Duration // per-action context timeout (default 30s) + idleTimeout time.Duration // auto-close pages idle longer than this (default 10m, 0=disabled) + maxPages int // max open pages per tenant (default 5) + stopReaper chan struct{} // signal to stop the reaper goroutine + logger *slog.Logger } // Option configures a Manager. @@ -43,15 +49,34 @@ func WithLogger(l *slog.Logger) Option { return func(m *Manager) { m.logger = l } } +// WithActionTimeout sets the per-action context timeout. +func WithActionTimeout(d time.Duration) Option { + return func(m *Manager) { m.actionTimeout = d } +} + +// WithIdleTimeout sets the idle page auto-close timeout. 0 disables the reaper. +func WithIdleTimeout(d time.Duration) Option { + return func(m *Manager) { m.idleTimeout = d } +} + +// WithMaxPages sets the max open pages per tenant. +func WithMaxPages(n int) Option { + return func(m *Manager) { m.maxPages = n } +} + // New creates a Manager with options. func New(opts ...Option) *Manager { m := &Manager{ - refs: NewRefStore(), - pages: make(map[string]*rod.Page), - console: make(map[string][]ConsoleMessage), - tenantCtxs: make(map[string]*rod.Browser), - pageTenants: make(map[string]string), - logger: slog.Default(), + refs: NewRefStore(), + pages: make(map[string]*rod.Page), + console: make(map[string][]ConsoleMessage), + tenantCtxs: make(map[string]*rod.Browser), + pageTenants: make(map[string]string), + pageLastUsed: make(map[string]time.Time), + actionTimeout: 30 * time.Second, + idleTimeout: 10 * time.Minute, + maxPages: 5, + logger: slog.Default(), } for _, o := range opts { o(m) @@ -59,6 +84,16 @@ func New(opts ...Option) *Manager { return m } +// ActionTimeout returns the configured per-action timeout. +func (m *Manager) ActionTimeout() time.Duration { + return m.actionTimeout +} + +// touchPageLocked updates the last-used timestamp for a page. Must be called with mu held. +func (m *Manager) touchPageLocked(targetID string) { + m.pageLastUsed[targetID] = time.Now() +} + // Start launches a local Chrome browser or connects to a remote one. // If already connected but the connection is dead, it reconnects automatically. func (m *Manager) Start(ctx context.Context) error { @@ -112,11 +147,28 @@ func (m *Manager) Start(ctx context.Context) error { } m.browser = b + + // Start idle-page reaper if configured + if m.idleTimeout > 0 && m.stopReaper == nil { + m.stopReaper = make(chan struct{}) + go m.runReaper() + } + return nil } // Stop closes the Chrome browser (local) or disconnects (remote sidecar). func (m *Manager) Stop(ctx context.Context) error { + // Grab and nil-out stopReaper under the lock, then close outside to avoid + // deadlock (reaper goroutine also acquires mu). + m.mu.Lock() + ch := m.stopReaper + m.stopReaper = nil + m.mu.Unlock() + if ch != nil { + close(ch) + } + m.mu.Lock() defer m.mu.Unlock() @@ -137,6 +189,7 @@ func (m *Manager) Stop(ctx context.Context) error { m.pages = make(map[string]*rod.Page) m.console = make(map[string][]ConsoleMessage) m.pageTenants = make(map[string]string) + m.pageLastUsed = make(map[string]time.Time) return err } diff --git a/pkg/browser/browser_reaper.go b/pkg/browser/browser_reaper.go new file mode 100644 index 00000000..010fa21e --- /dev/null +++ b/pkg/browser/browser_reaper.go @@ -0,0 +1,54 @@ +package browser + +import "time" + +// runReaper periodically closes pages that have been idle longer than idleTimeout. +// Runs as a goroutine; exits when stopReaper is closed. +func (m *Manager) runReaper() { + ticker := time.NewTicker(60 * time.Second) + defer ticker.Stop() + + for { + select { + case <-m.stopReaper: + return + case <-ticker.C: + m.reapIdlePages() + } + } +} + +// reapIdlePages closes pages idle longer than idleTimeout. +func (m *Manager) reapIdlePages() { + m.mu.Lock() + defer m.mu.Unlock() + + if m.browser == nil { + return + } + + now := time.Now() + for targetID, lastUsed := range m.pageLastUsed { + if now.Sub(lastUsed) <= m.idleTimeout { + continue + } + + page, ok := m.pages[targetID] + if !ok { + delete(m.pageLastUsed, targetID) + continue + } + + if err := page.Close(); err != nil { + m.logger.Warn("reaper: failed to close idle page", "targetId", targetID, "error", err) + continue + } + + delete(m.pages, targetID) + delete(m.console, targetID) + delete(m.pageTenants, targetID) + delete(m.pageLastUsed, targetID) + m.refs.Remove(targetID) + m.logger.Info("reaper: closed idle page", "targetId", targetID, "idle", now.Sub(lastUsed).Round(time.Second)) + } +} diff --git a/pkg/browser/browser_remote.go b/pkg/browser/browser_remote.go index a2a339f9..9e713e62 100644 --- a/pkg/browser/browser_remote.go +++ b/pkg/browser/browser_remote.go @@ -22,6 +22,7 @@ func (m *Manager) reconnectLocked() error { m.pages = make(map[string]*rod.Page) m.console = make(map[string][]ConsoleMessage) m.pageTenants = make(map[string]string) + m.pageLastUsed = make(map[string]time.Time) m.refs = NewRefStore() controlURL, err := resolveRemoteCDP(m.remoteURL) @@ -100,6 +101,11 @@ func (m *Manager) getPageForTenant(targetID, tenantID string) (*rod.Page, error) } // If no tenant context or master tenant, allow access to all pages if tenantID == "" || tenantID == MasterTenantID { + resolvedTID := targetID + if targetID == "" { + resolvedTID = string(page.TargetID) + } + m.touchPageLocked(resolvedTID) return page, nil } // Check ownership: page must belong to this tenant @@ -110,6 +116,7 @@ func (m *Manager) getPageForTenant(targetID, tenantID string) (*rod.Page, error) if owner, ok := m.pageTenants[resolvedTID]; ok && owner != tenantID { return nil, fmt.Errorf("tab not found: %s", targetID) } + m.touchPageLocked(resolvedTID) return page, nil } diff --git a/pkg/browser/browser_tabs.go b/pkg/browser/browser_tabs.go index 08c138e1..e601044b 100644 --- a/pkg/browser/browser_tabs.go +++ b/pkg/browser/browser_tabs.go @@ -68,11 +68,18 @@ func (m *Manager) ListTabs(ctx context.Context) ([]TabInfo, error) { // OpenTab opens a new tab with the given URL. // Pages are created within the tenant's incognito browser context for isolation. +// If the tenant already has maxPages open, the oldest idle page is closed first. func (m *Manager) OpenTab(ctx context.Context, url string) (*TabInfo, error) { m.mu.Lock() defer m.mu.Unlock() tenantID := tenantIDFromCtx(ctx) + + // Enforce max pages per tenant + if m.maxPages > 0 { + m.evictOldestIfOverLimitLocked(tenantID) + } + b, err := m.tenantBrowserLocked(tenantID) if err != nil { return nil, err @@ -89,6 +96,7 @@ func (m *Manager) OpenTab(ctx context.Context, url string) (*TabInfo, error) { info, _ := page.Info() tid := string(page.TargetID) m.pages[tid] = page + m.touchPageLocked(tid) if tenantID != "" { m.pageTenants[tid] = tenantID } @@ -104,6 +112,60 @@ func (m *Manager) OpenTab(ctx context.Context, url string) (*TabInfo, error) { return tab, nil } +// evictOldestIfOverLimitLocked closes the oldest idle page for a tenant if at or over maxPages. +// Must be called with mu held. +func (m *Manager) evictOldestIfOverLimitLocked(tenantID string) { + isMaster := tenantID == "" || tenantID == MasterTenantID + + // Collect targetIDs belonging to this tenant + var owned []string + for tid := range m.pages { + if isMaster { + // Master tenant owns pages not in pageTenants + if _, hasOwner := m.pageTenants[tid]; !hasOwner { + owned = append(owned, tid) + } + } else { + if m.pageTenants[tid] == tenantID { + owned = append(owned, tid) + } + } + } + + if len(owned) < m.maxPages { + return + } + + // Find the oldest page by lastUsed + var oldestID string + var oldestTime time.Time + for _, tid := range owned { + lu, ok := m.pageLastUsed[tid] + if !ok { + oldestID = tid + break + } + if oldestID == "" || lu.Before(oldestTime) { + oldestID = tid + oldestTime = lu + } + } + + if oldestID == "" { + return + } + + if page, ok := m.pages[oldestID]; ok { + _ = page.Close() + } + delete(m.pages, oldestID) + delete(m.console, oldestID) + delete(m.pageTenants, oldestID) + delete(m.pageLastUsed, oldestID) + m.refs.Remove(oldestID) + m.logger.Info("evicted oldest page (max pages reached)", "targetId", oldestID, "tenant", tenantID) +} + // FocusTab activates a tab. func (m *Manager) FocusTab(ctx context.Context, targetID string) error { tenantID := tenantIDFromCtx(ctx) @@ -133,6 +195,8 @@ func (m *Manager) CloseTab(ctx context.Context, targetID string) error { delete(m.pages, targetID) delete(m.console, targetID) delete(m.pageTenants, targetID) + delete(m.pageLastUsed, targetID) + m.refs.Remove(targetID) return page.Close() } diff --git a/pkg/browser/refs.go b/pkg/browser/refs.go index ff26afd9..49b017f6 100644 --- a/pkg/browser/refs.go +++ b/pkg/browser/refs.go @@ -81,6 +81,14 @@ func NormalizeRef(raw string) string { return s } +// Remove deletes all refs for a target. +func (rs *RefStore) Remove(targetID string) { + rs.mu.Lock() + defer rs.mu.Unlock() + rs.removeFromOrder(targetID) + delete(rs.entries, targetID) +} + func (rs *RefStore) removeFromOrder(targetID string) { for i, id := range rs.order { if id == targetID { diff --git a/pkg/browser/tool.go b/pkg/browser/tool.go index 08b6efba..cc48b80e 100644 --- a/pkg/browser/tool.go +++ b/pkg/browser/tool.go @@ -151,6 +151,18 @@ func (t *BrowserTool) Execute(ctx context.Context, args map[string]any) *tools.R } } + // Apply per-action timeout for heavy operations + switch action { + case "open", "navigate", "snapshot", "screenshot", "act": + timeout := t.manager.ActionTimeout() + if ms, ok := args["timeoutMs"].(float64); ok && ms > 0 { + timeout = time.Duration(ms) * time.Millisecond + } + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, timeout) + defer cancel() + } + switch action { case "status": return t.handleStatus()