Conversation
|
agents
@cloudflare/ai-chat
@cloudflare/codemode
hono-agents
@cloudflare/shell
@cloudflare/think
@cloudflare/voice
@cloudflare/worker-bundler
commit: |
5a1519c to
671c189
Compare
1cd5d17 to
440148c
Compare
🟡 agents import sizesMeasured 343 runtime imports as minified bundles. The primary size is gzip; raw minified size is included for diagnosis. An existing import growing by more than 10% is marked red. This report is informational.
Compared Changed imports (10)
All 343 current runtime imports
Reported by agent-think[bot]. |
671c189 to
41c4b81
Compare
| await this.#store.set(key, { ...current, closedAt: Date.now() }); | ||
| } finally { | ||
| await lock.release(); | ||
| } | ||
| await deleteBrowserSession(this.#browser, stored.sessionId); |
There was a problem hiding this comment.
🟡 Deletion failures become unretryable
When deletion fails after close() tombstones a session, later closes return false without retrying. sweep() treats the tombstone as deleted, leaving the browser alive until platform expiry.
Learn more
A tombstone currently represents both “closure requested” and “platform deletion completed.” close() writes that state before making the delete request. If the request fails, the tombstone branch only ages and prunes the record; it never retries deletion. The sweep failure path has the same state transition before its best-effort delete.
Example: close("checkout") tombstones session-7, then Browser Run returns 500 to the delete. Retrying close("checkout") returns false, and future sweeps eventually remove the tombstone without another delete request.
Recommended fix: Persist a deletion-pending state and retry deletion from close() and sweep() until a successful or 404 response. Only transition to a completed tombstone after deletion succeeds, while retaining the restart evidence required by resolve().
Was this helpful? React with 👍 or 👎 to provide feedback.
c670fa0 to
2e30e9f
Compare
| if (winner) { | ||
| try { | ||
| await deleteBrowserSession(this.#browser, stored.sessionId); | ||
| } catch (error) { | ||
| console.warn( | ||
| `[agents/browser] Failed to delete redundant Browser Run session ${stored.sessionId}`, | ||
| error | ||
| ); | ||
| } | ||
| return { winner }; |
There was a problem hiding this comment.
🟡 Concurrent closure returns a dead session
If close() or sweep() tombstones the winner during redundant-session cleanup, resolve() returns its stale live snapshot. The returned session was already deleted, so attachment fails.
Learn more
A create race stores current in winner, releases the lock, and awaits deletion of the redundant platform session. The stored entry can change while that network request runs. The caller only checks winner.closedAt on the old object at resolve, so a concurrent tombstone is invisible.
Example: Two resolvers replace the same tombstone. Resolver A commits session A. Resolver B records session A as the winner and starts deleting session B. A concurrent close() tombstones and deletes session A before B's cleanup finishes. Resolver B still returns session A as live.
Recommended fix: Re-read and validate the key after redundant-session cleanup before returning a winner. Retry resolution when the current entry differs, is absent, or is tombstoned.
Was this helpful? React with 👍 or 👎 to provide feedback.
Provider-independent named sessions over the existing store contract: - NamedBrowserSessions.resolve() is reattach-or-create keyed by browser:session:<name> — hosts wire names, models never see session identity. restarted: true signals loudly whenever a prior browser was lost (died upstream, closed, or swept); first-ever use is not a restart. - Durable creation options (keepAliveMs, recording, guardrails.allowedDomains/allowedDomainSets) are reapplied on every create so they survive the restart path. keep_alive pins to the 600-second platform max by default. - createBrowserSession() gains guardrails support — they ride the acquire POST body per the Browser Run REST contract; Kitesurf rejects them. - close() tombstones and deletes; sweep() closes idle sessions, leaves tombstones, and prunes aged ones (creator-overridable 10-minute TTL, alarm scheduling lands with the Lifecycle capability slice). - openOneShotBrowserSession() is the trivial create-and-close branch: no store, platform session deleted when the CDP socket closes. connectBrowserSession() gains an optional onClose to support it. Lock discipline mirrors BrowserConnector: locks wrap storage only, probes and Browser Run calls run outside, commits re-check for concurrent swaps (covered by tests, including a lock-across-network violation detector). Internal module — no public export changes until the starter-pack slice. Existing browser_execute behavior untouched.
2e30e9f to
5b4dad0
Compare
|
/devin review |
What this is
A session engine. The host names a browser (e.g.
"checkout","research") and one call does the right thing:Internal module (
browser/session-core.ts), plus two small backward-compatible additions to already-public helpers - this is the only public behaviour change in this stack.Why
A Browser Rendering session is a real Chrome browser: slow to start, and it forgets logins, cookies, and tabs when it dies. Today, keeping one alive across model turns means the model does the bookkeeping — deciding survival with
cdp.startSession()and carrying raw protocol ids between turns — and evaluation showed it keeps fumbling that (stale-id failures, attach ceremony). This engine moves all of it host-side:restarted: truereplacesCDP error -32001; first-ever use is not a restart.keepAliveMs,recording, andguardrailsare stored durably and reapplied to every replacement browser. Keep-alive defaults to the 600-second platform max.close()tombstones then deletes (best-effort — the tombstone is the durable outcome, and the pinnedkeep_alivereclaims any browser whose delete failed);sweep()closes idle browsers and prunes aged tombstones (10-minute default; alarm scheduling lands in the next PR).connect()refresh the session's idle clock (throttled to 60s, capped at half the sweep window so a short customsweepIdleMscannot outrun the touch cadence), so a sweep only closes browsers that are genuinely idle.restarted: trueinstead of mistaking it for first use.openOneShotBrowserSession()is store-less create-use-delete. Kitesurf will use this option.Storage discipline mirrors the existing connector: locks wrap storage reads/writes only, never Browser Run network calls; concurrent create races resolve first-commit-wins with best-effort cleanup of the loser. The tests include a detector that fails if any network call happens while a lock is held.
Public API impact
Two additions to existing
agents/browserexports, both backward compatible:createBrowserSession(browser, { keepAliveMs, includeTargets, recording, + guardrails: { allowedDomains: [...], allowedDomainSets: [...] } }) -connectBrowserSession(browser, sessionId, timeoutMs?) +connectBrowserSession(browser, sessionId, { timeoutMs?, onClose?, onActivity? })The bare-number
timeoutMsform still works but is marked@deprecated(overload-level, so editors flag it) — the options object is the canonical form and internal callers are migrated to it.guardrailsare just an addition to the existing POST body to acquire a Browser Run session (recently added to their REST contract) and are fixed for the session's life, every connection included. The type names are exported (with the changeset) in the docs PR at the top of the stack.onClose, run exactly once when the session reaches a terminal state — explicit close, peer closure, or socket erroronActivity, invoked on every CDP command sent — the activity signal the session core uses for idle trackingThe engine uses a new
browser:session:<name>keyspace, disjoint from the connector'scdp:exec:/cdp:reuse:entries — the two coexist, andbrowser_executebehavior is untouched.Reviewing
session-core.tsreads top to bottom: resolve → connect → close → sweep → one-shot.browser-session-core.test.ts: first-create, live reattach, dead/tombstoned recreate, a concurrent dead-recovery race (both resolvers reportrestarted: true, first-commit-wins, redundant browser reclaimed), option reapplication, per-name isolation, lock discipline, activity touches (refresh, throttle, derived interval under a short sweep window, no resurrection of closed sessions), sweep behavior, close with a failed platform delete, one-shot cleanup including the attach-failure and peer-closure paths plus the dispose-once guard, Kitesurf rejection.